Skip to content

Commit dbd896d

Browse files
juliusmarmingeBilalIrfanContactclaude
committed
fix(checkpoint): fetched upstream commits no longer show as agent changes
A turn that only fetched and fast-forwarded listed the incoming files as its own whenever their commits were newer than the previous checkpoint, because the classifier judged a commit by its committer time alone. A commit now counts as imported when a remote-tracking reflog shows a fetch delivering it since the previous checkpoint and no reflog in any worktree shows this repository creating it. The reflog walk uses --all, so the primary worktree's HEAD and every linked worktree's HEAD count as local evidence. Without usable reflogs the old behaviour stays. Fixes #25452 Co-authored-by: Bilal Irfan <[email protected]> Co-Authored-By: Claude Fable 5.2 <[email protected]>
1 parent 9efd960 commit dbd896d

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("keeps paths from signed commits under log.showSignature", () =>
550601
Effect.gen(function* () {
551602
const tmp = yield* makeTmpDir();

‎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",
@@ -1586,7 +1590,17 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
15861590
return null;
15871591
}
15881592

1589-
const listPaths = (args: ReadonlyArray<string>) =>
1593+
const listLines = (args: ReadonlyArray<string>, stdin?: string) =>
1594+
execute({
1595+
operation,
1596+
cwd: input.cwd,
1597+
// log.showSignature prints verification text into the output.
1598+
args: ["-c", "log.showSignature=false", ...args],
1599+
...(stdin === undefined ? {} : { stdin }),
1600+
maxOutputBytes: VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
1601+
outputMode: "error",
1602+
}).pipe(Effect.map((result) => result.stdout.split("\n").filter((line) => line !== "")));
1603+
const listPaths = (args: ReadonlyArray<string>, stdin?: string) =>
15901604
execute({
15911605
operation,
15921606
cwd: input.cwd,
@@ -1602,26 +1616,73 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
16021616
"--no-renames",
16031617
"--no-ext-diff",
16041618
],
1619+
...(stdin === undefined ? {} : { stdin }),
16051620
maxOutputBytes: VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
16061621
outputMode: "error",
16071622
}).pipe(Effect.map((result) => result.stdout.split("\0")));
1623+
1624+
// A commit is an import when a fetch delivered it since the previous checkpoint and no
1625+
// reflog in any worktree shows this repository creating it. --all covers every worktree's
1626+
// HEAD, and with --walk-reflogs --since compares each entry's own time.
1627+
const listImportedCommits = Effect.gen(function* () {
1628+
const entries = (yield* listLines([
1629+
"log",
1630+
"--walk-reflogs",
1631+
"--all",
1632+
`--since=@${capturedAt}`,
1633+
"--format=%H %gD %gs",
1634+
])).map((line) => {
1635+
const [commit = "", selector = "", ...subject] = line.split(" ");
1636+
return {
1637+
commit,
1638+
fetched: selector.startsWith("refs/remotes/"),
1639+
subject: subject.join(" "),
1640+
};
1641+
});
1642+
// A push records commits this repository already had.
1643+
const fetchedTips = new Set(
1644+
entries
1645+
.filter((entry) => entry.fetched && entry.subject !== "update by push")
1646+
.map((entry) => entry.commit),
1647+
);
1648+
if (fetchedTips.size === 0) return fetchedTips;
1649+
// Fetching another branch can deliver a commit that was made here and pushed.
1650+
const createdHere = new Set(
1651+
entries
1652+
.filter((entry) => !entry.fetched && !REF_MOVE_REFLOG_SUBJECT.test(entry.subject))
1653+
.map((entry) => entry.commit),
1654+
);
1655+
const fetched = yield* listLines(
1656+
["rev-list", `--since=@${capturedAt}`, "--stdin"],
1657+
`${[...fetchedTips, `^${startHead}`].join("\n")}\n`,
1658+
);
1659+
return new Set(fetched.filter((commit) => !createdHere.has(commit)));
1660+
});
1661+
16081662
// Remerge diffs retain merge conflict fixes while excluding clean upstream changes.
1609-
const pathLists = yield* Effect.all(
1663+
const [recentCommits, ...pathLists] = yield* Effect.all(
16101664
[
1665+
listLines(["rev-list", `--since=@${capturedAt}`, `${startHead}..${endHead}`]),
16111666
listPaths(["diff", startHead, `${input.fromCheckpointRef}^{commit}`]),
16121667
listPaths(["diff", endHead, `${input.toCheckpointRef}^{commit}`]),
16131668
listPaths(["log", "--format=", "--diff-merges=remerge", `${endHead}..${startHead}`]),
1614-
listPaths([
1615-
"log",
1616-
"--format=",
1617-
"--diff-merges=remerge",
1618-
`--since=@${capturedAt}`,
1619-
`${startHead}..${endHead}`,
1620-
]),
16211669
],
16221670
{ concurrency: "unbounded" },
16231671
);
1624-
return new Set(pathLists.flat().filter((path) => path.length > 0));
1672+
// Without readable reflogs every recent commit stays attributed, as before.
1673+
const importedCommits =
1674+
recentCommits.length === 0
1675+
? new Set<string>()
1676+
: yield* listImportedCommits.pipe(Effect.orElseSucceed(() => new Set<string>()));
1677+
const ownCommits = recentCommits.filter((commit) => !importedCommits.has(commit));
1678+
const ownCommitPaths =
1679+
ownCommits.length === 0
1680+
? []
1681+
: yield* listPaths(
1682+
["log", "--no-walk=unsorted", "--stdin", "--format=", "--diff-merges=remerge"],
1683+
`${ownCommits.join("\n")}\n`,
1684+
);
1685+
return new Set([...pathLists.flat(), ...ownCommitPaths].filter((path) => path.length > 0));
16251686
}),
16261687

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

0 commit comments

Comments
 (0)