Skip to content

Commit 9402dc3

Browse files
fix(checkpoint): fetched upstream commits no longer show as agent changes (#36332)
Co-authored-by: Bilal Irfan <[email protected]>
1 parent 0a6b3eb commit 9402dc3

2 files changed

Lines changed: 122 additions & 10 deletions

File tree

‎apps/server/src/checkpointing/CheckpointStore.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,57 @@ it.layer(layerTest)("CheckpointStore.layer", (it) => {
546546
}),
547547
);
548548

549+
it.effect("leaves out fetched commits newer than the checkpoint, keeping local ones", () =>
550+
Effect.gen(function* () {
551+
const root = yield* makeTmpDir();
552+
const upstream = NodePath.join(root, "upstream");
553+
const primary = NodePath.join(root, "primary");
554+
const cwd = NodePath.join(root, "turn");
555+
yield* (yield* FileSystem.FileSystem).makeDirectory(upstream);
556+
yield* initRepoWithCommit(upstream);
557+
yield* git(root, ["clone", upstream, primary]);
558+
yield* git(primary, ["config", "user.email", "[email protected]"]);
559+
yield* git(primary, ["config", "user.name", "Test"]);
560+
yield* git(primary, ["worktree", "add", cwd, "-b", "turn"]);
561+
const checkpointStore = yield* CheckpointStore.CheckpointStore;
562+
const threadId = ThreadId.make("checkpoint-authored-paths-fetched");
563+
const refs = {
564+
cwd,
565+
fromCheckpointRef: checkpointRefForThreadTurn(threadId, 0),
566+
toCheckpointRef: checkpointRefForThreadTurn(threadId, 1),
567+
};
568+
yield* checkpointStore.captureCheckpoint({ cwd, checkpointRef: refs.fromCheckpointRef });
569+
570+
const commitFile = (repo: string, file: string) =>
571+
Effect.gen(function* () {
572+
yield* writeTextFile(NodePath.join(repo, file), `${file}\n`);
573+
yield* git(repo, ["add", "."]);
574+
yield* git(repo, ["commit", "-m", file]);
575+
});
576+
577+
yield* git(upstream, ["checkout", "-b", "release"]);
578+
yield* commitFile(upstream, "first.txt");
579+
yield* git(cwd, ["fetch", "origin"]);
580+
yield* commitFile(upstream, "second.txt");
581+
// The primary worktree's HEAD reflog is the only record that this commit was made here.
582+
yield* git(primary, ["checkout", "--detach"]);
583+
yield* commitFile(primary, "own.txt");
584+
yield* git(primary, ["push", "origin", "HEAD:refs/heads/pushed"]);
585+
yield* git(upstream, ["merge", "pushed", "--no-edit"]);
586+
587+
yield* git(cwd, ["fetch", "origin"]);
588+
yield* git(cwd, ["merge", "--ff-only", "origin/release"]);
589+
yield* commitFile(cwd, "local.txt");
590+
// Pushing imported commits to another branch must not make them look local.
591+
yield* git(cwd, ["push", "origin", "HEAD:refs/heads/mirror"]);
592+
yield* checkpointStore.captureCheckpoint({ cwd, checkpointRef: refs.toCheckpointRef });
593+
594+
expect(yield* checkpointStore.listAuthoredPaths(refs)).toEqual(
595+
new Set(["local.txt", "own.txt"]),
596+
);
597+
}),
598+
);
599+
549600
it.effect.each([{ committed: "before the turn" }, { committed: "during the turn" }])(
550601
"starts a turn that switched branches from the branch it landed on (committed $committed)",
551602
({ committed }) =>

‎apps/server/src/vcs/GitVcsDriver.ts‎

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,10 @@ const FLAGGED_INDEX_ENTRIES_MAX_BYTES = 1024 * 1024;
467467
const CHECKPOINT_RECOVERY_MAX_CANDIDATES = 64;
468468
const CHECKPOINT_RECOVERY_TIMEOUT = "5 seconds";
469469
const GIT_CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024;
470+
// Reflog subjects that only point a ref at an existing commit. Anything else counts as creating
471+
// one, so an unknown subject keeps its commit attributed.
472+
const REF_MOVE_REFLOG_SUBJECT =
473+
/^(?:$|(?:checkout|reset|branch|clone|fetch)\b|[^:]*\((?:start|finish|abort|reset)\): |(?:merge|pull|rebase|cherry-pick)\b[^()]*: fast-forward$)/i;
470474
const WORKSPACE_GIT_HARDENED_CONFIG_ARGS = [
471475
"-c",
472476
"core.fsmonitor=false",
@@ -1858,7 +1862,17 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
18581862
return null;
18591863
}
18601864

1861-
const listPaths = (args: ReadonlyArray<string>) =>
1865+
const listLines = (args: ReadonlyArray<string>, stdin?: string) =>
1866+
execute({
1867+
operation,
1868+
cwd: input.cwd,
1869+
// log.showSignature prints verification text into the output.
1870+
args: ["-c", "log.showSignature=false", ...args],
1871+
...(stdin === undefined ? {} : { stdin }),
1872+
maxOutputBytes: VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
1873+
outputMode: "error",
1874+
}).pipe(Effect.map((result) => result.stdout.split("\n").filter((line) => line !== "")));
1875+
const listPaths = (args: ReadonlyArray<string>, stdin?: string) =>
18621876
execute({
18631877
operation,
18641878
cwd: input.cwd,
@@ -1874,26 +1888,73 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
18741888
"--no-renames",
18751889
"--no-ext-diff",
18761890
],
1891+
...(stdin === undefined ? {} : { stdin }),
18771892
maxOutputBytes: VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
18781893
outputMode: "error",
18791894
}).pipe(Effect.map((result) => result.stdout.split("\0")));
1895+
1896+
// A commit is an import when a fetch delivered it since the previous checkpoint and no
1897+
// reflog in any worktree shows this repository creating it. --all covers every worktree's
1898+
// HEAD, and with --walk-reflogs --since compares each entry's own time.
1899+
const listImportedCommits = Effect.gen(function* () {
1900+
const entries = (yield* listLines([
1901+
"log",
1902+
"--walk-reflogs",
1903+
"--all",
1904+
`--since=@${capturedAt}`,
1905+
"--format=%H %gD %gs",
1906+
])).map((line) => {
1907+
const [commit = "", selector = "", ...subject] = line.split(" ");
1908+
return {
1909+
commit,
1910+
fetched: selector.startsWith("refs/remotes/"),
1911+
subject: subject.join(" "),
1912+
};
1913+
});
1914+
// A push records commits this repository already had.
1915+
const fetchedTips = new Set(
1916+
entries
1917+
.filter((entry) => entry.fetched && entry.subject !== "update by push")
1918+
.map((entry) => entry.commit),
1919+
);
1920+
if (fetchedTips.size === 0) return fetchedTips;
1921+
// Fetching another branch can deliver a commit that was made here and pushed.
1922+
const createdHere = new Set(
1923+
entries
1924+
.filter((entry) => !entry.fetched && !REF_MOVE_REFLOG_SUBJECT.test(entry.subject))
1925+
.map((entry) => entry.commit),
1926+
);
1927+
const fetched = yield* listLines(
1928+
["rev-list", `--since=@${capturedAt}`, "--stdin"],
1929+
`${[...fetchedTips, `^${startHead}`].join("\n")}\n`,
1930+
);
1931+
return new Set(fetched.filter((commit) => !createdHere.has(commit)));
1932+
});
1933+
18801934
// Remerge diffs retain merge conflict fixes while excluding clean upstream changes.
1881-
const pathLists = yield* Effect.all(
1935+
const [recentCommits, ...pathLists] = yield* Effect.all(
18821936
[
1937+
listLines(["rev-list", `--since=@${capturedAt}`, `${startHead}..${endHead}`]),
18831938
listPaths(["diff", startHead, `${turnStart}^{commit}`]),
18841939
listPaths(["diff", endHead, `${input.toCheckpointRef}^{commit}`]),
18851940
listPaths(["log", "--format=", "--diff-merges=remerge", `${endHead}..${startHead}`]),
1886-
listPaths([
1887-
"log",
1888-
"--format=",
1889-
"--diff-merges=remerge",
1890-
`--since=@${capturedAt}`,
1891-
`${startHead}..${endHead}`,
1892-
]),
18931941
],
18941942
{ concurrency: "unbounded" },
18951943
);
1896-
return new Set(pathLists.flat().filter((path) => path.length > 0));
1944+
// Without readable reflogs every recent commit stays attributed, as before.
1945+
const importedCommits =
1946+
recentCommits.length === 0
1947+
? new Set<string>()
1948+
: yield* listImportedCommits.pipe(Effect.orElseSucceed(() => new Set<string>()));
1949+
const ownCommits = recentCommits.filter((commit) => !importedCommits.has(commit));
1950+
const ownCommitPaths =
1951+
ownCommits.length === 0
1952+
? []
1953+
: yield* listPaths(
1954+
["log", "--no-walk=unsorted", "--stdin", "--format=", "--diff-merges=remerge"],
1955+
`${ownCommits.join("\n")}\n`,
1956+
);
1957+
return new Set([...pathLists.flat(), ...ownCommitPaths].filter((path) => path.length > 0));
18971958
}),
18981959

18991960
deleteCheckpointRefs: Effect.fn("GitVcsDriver.checkpoints.deleteCheckpointRefs")(

0 commit comments

Comments
 (0)