Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions apps/server/src/git/linkCreatedPullRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ it.effect(
"refreshes PR readers after a push from a thread or project, but not a local commit",
() =>
Effect.gen(function* () {
const refreshed: string[] = [];
const refreshed: Array<[string, boolean | undefined]> = [];
const layerDependencies = Layer.mergeAll(
Layer.mock(Orchestrator.OrchestratorV2)({
getThreadShell: () => Effect.succeed(v2PullRequestThread(thread)),
Expand All @@ -178,9 +178,9 @@ it.effect(
listShells: () => Effect.succeed([project]),
}),
Layer.mock(PullRequestService.PullRequestService)({
refreshAfterTurn: (id) =>
refreshAfterTurn: (id, options) =>
Effect.sync(() => {
refreshed.push(id);
refreshed.push([id, options?.pushed]);
}),
}),
);
Expand All @@ -203,6 +203,10 @@ it.effect(
{ cwd: "/draft-worktree", projectId: PROJECT_ID },
{ push: { status: "pushed" } },
).pipe(Effect.provide(layerDependencies));
expect(refreshed).toEqual([PROJECT_ID, PROJECT_ID, PROJECT_ID]);
expect(refreshed).toEqual([
[PROJECT_ID, true],
[PROJECT_ID, true],
[PROJECT_ID, true],
]);
}),
);
6 changes: 3 additions & 3 deletions apps/server/src/git/refreshPushedPullRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,18 @@ export const refreshPushedPullRequests = Effect.fn("refreshPushedPullRequests")(
const engine = yield* OrchestratorV2.OrchestratorV2;
const thread = yield* engine.getThreadShell(input.threadId);
if (thread !== null) {
yield* pullRequests.refreshAfterTurn(thread.projectId);
yield* pullRequests.refreshAfterTurn(thread.projectId, { pushed: true });
return;
}
}
if (input.projectId !== undefined) {
yield* pullRequests.refreshAfterTurn(input.projectId);
yield* pullRequests.refreshAfterTurn(input.projectId, { pushed: true });
return;
}
const projects = yield* (yield* ProjectStore.ProjectStoreV2).listShells();
yield* Effect.forEach(
projects.filter((project) => project.workspaceRoot === input.cwd),
(project) => pullRequests.refreshAfterTurn(project.id),
(project) => pullRequests.refreshAfterTurn(project.id, { pushed: true }),
{ discard: true },
);
},
Expand Down
34 changes: 34 additions & 0 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2671,6 +2671,40 @@ it.effect("does not revive old summaries when project epochs are evicted", () =>
}),
);

it.effect("a finished turn leaves another project's cached listing in place", () =>
Effect.gen(function* () {
const reads: string[] = [];
const service = yield* makeService({
projects: [
project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }),
project({ id: "p2", title: "api", workspaceRoot: "/b", repository: "acme/api" }),
],
providers: [
fakePullRequestProvider(SourceControlProviderKind.make("github"), {
listChangeRequests: ({ repository }) => {
reads.push(repository);
return Effect.succeed({ items: [], truncated: false, continues: false });
},
}),
],
});
const webListing = { state: "open" as const, projectId: "p1" as ProjectId };
const everyListing = { state: "open" as const };

yield* service.list(webListing);
yield* service.list(everyListing);
reads.length = 0;
yield* service.refreshAfterTurn("p2" as ProjectId);
yield* service.list(webListing);
yield* service.list(everyListing);
assert.deepStrictEqual(reads, []);

yield* service.refreshAfterTurn("p2" as ProjectId, { pushed: true });
yield* service.list(everyListing);
assert.deepStrictEqual(reads.toSorted(), ["acme/api", "acme/web"]);
}),
);

it.effect("a refresh or a finished turn drops the provider's own change request reads", () =>
Effect.gen(function* () {
const forgotten: Array<{ readonly repository: string; readonly number?: number } | undefined> =
Expand Down
14 changes: 9 additions & 5 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ export class PullRequestService extends Context.Service<
input: PullRequestReportStateInput,
) => Effect.Effect<void, PullRequestError>;
readonly subscribeRefreshes: Stream.Stream<number>;
readonly refreshAfterTurn: (projectId: ProjectId) => Effect.Effect<void>;
readonly refreshAfterTurn: (
projectId: ProjectId,
options?: { readonly pushed?: boolean },
) => Effect.Effect<void>;
/** Project and remote lookups inside `effect` read once per filter, e.g. across one sync sweep. */
readonly withSharedProjectReads: <A, E, R>(
effect: Effect.Effect<A, E, R>,
Expand Down Expand Up @@ -3639,12 +3642,14 @@ export const make = Effect.gen(function* () {

const refreshAfterTurn: PullRequestService["Service"]["refreshAfterTurn"] = Effect.fn(
"PullRequestService.refreshAfterTurn",
)(function* (projectId) {
)(function* (projectId, options) {
// Everything the provider holds, not just the project's repository: a turn can touch a
// change request reached through another repository, and no epoch may move until those
// reads are gone, or a reader in between keeps the old one under the new epoch.
yield* forgetProviderReads();
listingsEpoch = ++epochCounter;
const epoch = ++epochCounter;
// A push can open or move a PR; a plain turn waits for the listing TTL to spare the API quota.
if (options?.pushed === true) listingsEpoch = epoch;
projectEpochs.delete(projectId);
if (projectEpochs.size >= REF_EPOCH_CAPACITY) {
const oldest = projectEpochs.keys().next().value;
Expand All @@ -3653,8 +3658,7 @@ export const make = Effect.gen(function* () {
projectEpochs.delete(oldest);
}
}
projectEpochs.set(projectId, listingsEpoch);
const epoch = listingsEpoch;
projectEpochs.set(projectId, epoch);
yield* readCache.invalidate(`project:${projectId}`);
yield* SubscriptionRef.set(pullRequestRefreshes, epoch);
});
Expand Down
Loading