Skip to content

Commit deeaabc

Browse files
only21milclaude
andcommitted
fix(server): match stack siblings by repository as well as stack id
Stack ids are unique only within a repository: GitHub's decoder falls back to the stack number when the host sends no id. Two repositories that both report stack "3" with different layers flagged each other on every sweep and re-read both stacks forever. Siblings now match on the same normalized host, repository and stack id, the key the shared resolver already groups native stacks by. Refs #20370 Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 3ce9efa commit deeaabc

2 files changed

Lines changed: 51 additions & 1 deletion

File tree

‎apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,6 +1028,45 @@ describe("PullRequestSyncReactor", () => {
10281028
),
10291029
);
10301030

1031+
it.effect("leaves stacks in other repositories alone when their ids match", () =>
1032+
Effect.scoped(
1033+
Effect.gen(function* () {
1034+
yield* TestClock.setTime(Date.parse(NOW));
1035+
// Without a host id the GitHub decoder falls back to the stack number, so two
1036+
// repositories can both report stack "3".
1037+
const otherLink = makeLink(7, null, {
1038+
repository: "owner/other",
1039+
url: "https://github.com/owner/other/pull/7",
1040+
});
1041+
const fixture = yield* makeHarness({
1042+
snapshot: makeSnapshot([makeThread("one", { pullRequests: [makeLink(1), otherLink] })]),
1043+
stack: (input) =>
1044+
Effect.succeed({
1045+
id: "3",
1046+
number: 3,
1047+
url: `https://github.com/${input.repository}/stack/3`,
1048+
base: "main",
1049+
layers: [
1050+
{ number: input.number, headBranch: "layer-1", state: "open" },
1051+
{ number: input.number + 1, headBranch: "layer-2", state: "open" },
1052+
],
1053+
}),
1054+
});
1055+
1056+
yield* Effect.gen(function* () {
1057+
const reactor = yield* startAndSweep(fixture);
1058+
const firstSweep = yield* Ref.get(fixture.syncCommands);
1059+
yield* Ref.update(fixture.snapshots, (snapshot) => applySync(snapshot, firstSweep));
1060+
assert.strictEqual((yield* Ref.get(fixture.stackCalls)).length, 2);
1061+
1062+
yield* sweepAgain(fixture, reactor);
1063+
yield* sweepAgain(fixture, reactor);
1064+
assert.strictEqual((yield* Ref.get(fixture.stackCalls)).length, 2);
1065+
}).pipe(Effect.provide(fixture.layer));
1066+
}),
1067+
),
1068+
);
1069+
10311070
it.effect("keeps existing snapshots and continues when the host fails", () =>
10321071
Effect.scoped(
10331072
Effect.gen(function* () {

‎apps/server/src/orchestration-v2/PullRequestSyncReactor.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ function stacksEqual(
112112
);
113113
}
114114

115+
/** Stack ids are unique only within a repository, e.g. GitHub's fallback to the stack number. */
116+
function nativeStackKeyOf(link: ThreadPullRequestLink, stack: ThreadPullRequestStack): string {
117+
const key = normalizeThreadPullRequestKey(link);
118+
return `${key.host}/${key.repository}#stack:${stack.id}`;
119+
}
120+
115121
function sameLayerNumbers(left: ThreadPullRequestStack, right: ThreadPullRequestStack): boolean {
116122
return (
117123
left.layers.length === right.layers.length &&
@@ -326,12 +332,17 @@ export const make = Effect.gen(function* () {
326332
// would keep its shorter layer list. Members of this stack whose layer list differs read
327333
// their stack once, in this sweep if still pending or the next; agreeing ones read none.
328334
if (fetched !== null) {
335+
const fetchedKey = nativeStackKeyOf(first.link, fetched);
329336
for (const [siblingKey, siblingEntries] of groups) {
330337
if (siblingKey === key) continue;
331338
const readThisSweep = stacksReadThisSweep.get(siblingKey);
332339
const stale = siblingEntries.some(({ link }) => {
333340
const stack = readThisSweep === undefined ? link.stack : readThisSweep;
334-
return stack?.id === fetched.id && !sameLayerNumbers(stack, fetched);
341+
return (
342+
stack !== null &&
343+
nativeStackKeyOf(link, stack) === fetchedKey &&
344+
!sameLayerNumbers(stack, fetched)
345+
);
335346
});
336347
if (stale) retryStacks.add(siblingKey);
337348
}

0 commit comments

Comments
 (0)