Skip to content

Commit de37964

Browse files
juliusmarmingejuliusmarminge
authored
feat(prs): navigate, merge and rebase GitHub stacks (#10875)
1 parent f0401c6 commit de37964

53 files changed

Lines changed: 3104 additions & 464 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/mobile/src/features/threads/thread-work-log.tsx‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export const THREAD_DISCLOSURE_TRANSITION_MS = 180;
7676
const WORK_LOG_LAYOUT_TRANSITION = LinearTransition.duration(THREAD_DISCLOSURE_TRANSITION_MS);
7777
const WORK_LOG_DETAIL_ENTER_TRANSITION = FadeIn.duration(140);
7878
const WORK_LOG_DETAIL_EXIT_TRANSITION = FadeOut.duration(120);
79-
type WorkContentIcon = AppSymbolName | "browser" | "t3-code";
79+
type WorkContentIcon = AppSymbolName | "browser" | "t3-code" | "pull-request";
8080

8181
function WorkLogIcon(props: {
8282
readonly icon: WorkContentIcon;
@@ -92,7 +92,13 @@ function WorkLogIcon(props: {
9292
}
9393
return (
9494
<SymbolView
95-
name={props.icon === "browser" ? { ios: "globe", android: "public" } : props.icon}
95+
name={
96+
props.icon === "pull-request"
97+
? "arrow.triangle.pull"
98+
: props.icon === "browser"
99+
? { ios: "globe", android: "public" }
100+
: props.icon
101+
}
96102
size={14}
97103
weight="medium"
98104
{...(colorClassName ? { tintColorClassName: colorClassName } : { tintColor: props.color })}
@@ -893,7 +899,7 @@ export function ThreadWorkGroupToggle(props: {
893899
readonly iconSubtleColor: import("react-native").ColorValue;
894900
readonly summary: string;
895901
readonly summaryKind: ToolGroupSummaryKind;
896-
readonly summaryToolIcon?: "browser" | "t3-code";
902+
readonly summaryToolIcon?: "browser" | "t3-code" | "pull-request";
897903
readonly themeAppearance: "light" | "dark";
898904
readonly toolSurface?: import("@t3tools/contracts").ToolActivitySurface;
899905
readonly toolIcon?: ToolActivityIcon;
@@ -1231,6 +1237,11 @@ function ToolActivityImage(props: {
12311237

12321238
function toolGroupSummarySymbolName(kind: ToolGroupSummaryKind): AppSymbolName {
12331239
switch (kind) {
1240+
case "pull-request":
1241+
case "link-pr":
1242+
case "unlink-pr":
1243+
case "list-prs":
1244+
return "arrow.triangle.pull";
12341245
case "read":
12351246
return { ios: "eye", android: "visibility" };
12361247
case "edit":

‎apps/mobile/src/lib/threadActivity.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ export type ThreadFeedEntry =
167167
readonly summaryKind: ToolGroupSummaryKind;
168168
readonly toolSurface?: WorkLogEntry["toolSurface"];
169169
readonly toolIcon?: WorkLogEntry["toolIcon"];
170-
readonly summaryToolIcon?: "browser" | "t3-code";
170+
readonly summaryToolIcon?: "browser" | "t3-code" | "pull-request";
171171
readonly hasFailure: boolean;
172172
readonly live: boolean;
173173
readonly shimmer: boolean;

‎apps/server/src/environment/ServerEnvironment.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ export const make = Effect.gen(function* () {
231231
threadActiveReorder: true,
232232
threadTitleRegeneration: true,
233233
threadPullRequests: true,
234+
pullRequestStackActions: true,
234235
threadPullRequestLinking: true,
235236
environmentIcon: true,
236237
...(serverSelfUpdate === null ? {} : { serverSelfUpdate }),

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ const makeHarness = Effect.fn("makePullRequestSyncHarness")(function* (options:
178178
return yield* options.summary?.(input) ?? Effect.succeed(makeSummary(input));
179179
});
180180

181-
const stack: PullRequestService["Service"]["stack"] = (input) =>
181+
const stack: PullRequestService["Service"]["stack"] = (input, readOptions) =>
182182
Effect.gen(function* () {
183+
assert.strictEqual(readOptions?.includeDetails, false);
183184
yield* Ref.update(stackCalls, (calls) => [...calls, input]);
184185
return yield* options.stack?.(input) ?? Effect.succeed(null);
185186
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ export const make = Effect.gen(function* () {
256256
entry.link.snapshot === null || !snapshotFieldsEqual(entry.link.snapshot, fields),
257257
);
258258
const fetchedStack = needsStack
259-
? yield* pullRequests.stack(ref).pipe(
259+
? yield* pullRequests.stack(ref, { includeDetails: false }).pipe(
260260
Effect.map((stack) => ({
261261
stack: stack === null ? null : ({ kind: "native", ...stack } as const),
262262
})),

‎apps/server/src/orchestration/ThreadPullRequestReactor.test.ts‎

Lines changed: 121 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import {
2+
CheckpointRef,
23
EventId,
34
GitManagerError,
45
ProjectId,
56
ProviderInstanceId,
67
ThreadId,
8+
TurnId,
79
type OrchestrationCommand,
810
type OrchestrationEvent,
911
type OrchestrationProjectShell,
@@ -311,54 +313,79 @@ describe("ThreadPullRequestReactor", () => {
311313
),
312314
);
313315

314-
it.effect("refreshes discovery after a turn ends without client demand", () =>
315-
Effect.scoped(
316-
Effect.gen(function* () {
317-
const current = thread("turn-thread");
318-
const detected = yield* Ref.make<GitBranchPullRequest | null>(null);
319-
const fixture = yield* makeHarness({
320-
threads: [current],
321-
branchPullRequest: (_input, options) =>
322-
options?.refresh
323-
? Ref.set(detected, branchPullRequest()).pipe(Effect.andThen(Ref.get(detected)))
324-
: Ref.get(detected),
325-
});
326-
yield* Effect.gen(function* () {
327-
const reactor = yield* fixture.start();
328-
expect(yield* Ref.get(fixture.commands)).toHaveLength(0);
329-
yield* fixture.publish({
330-
type: "thread.session-set",
331-
sequence: 2,
332-
eventId: EventId.make("turn-finished"),
333-
aggregateKind: "thread",
334-
aggregateId: current.id,
335-
occurredAt: NOW,
336-
commandId: null,
337-
causationEventId: null,
338-
correlationId: null,
339-
metadata: {},
340-
payload: {
341-
threadId: current.id,
342-
session: {
316+
it.effect.each(["session-first", "checkpoint-first"] as const)(
317+
"refreshes discovery once after a turn ends with %s event ordering",
318+
(order) =>
319+
Effect.scoped(
320+
Effect.gen(function* () {
321+
const current = thread("turn-thread");
322+
const detected = yield* Ref.make<GitBranchPullRequest | null>(null);
323+
const fixture = yield* makeHarness({
324+
threads: [current],
325+
branchPullRequest: (_input, options) =>
326+
options?.refresh
327+
? Ref.set(detected, branchPullRequest()).pipe(Effect.andThen(Ref.get(detected)))
328+
: Ref.get(detected),
329+
});
330+
yield* Effect.gen(function* () {
331+
const reactor = yield* fixture.start();
332+
expect(yield* Ref.get(fixture.commands)).toHaveLength(0);
333+
const sessionEvent: OrchestrationEvent = {
334+
type: "thread.session-set",
335+
sequence: 2,
336+
eventId: EventId.make("turn-finished"),
337+
aggregateKind: "thread",
338+
aggregateId: current.id,
339+
occurredAt: NOW,
340+
commandId: null,
341+
causationEventId: null,
342+
correlationId: null,
343+
metadata: {},
344+
payload: {
343345
threadId: current.id,
346+
session: {
347+
threadId: current.id,
348+
status: "ready",
349+
providerName: "Codex",
350+
runtimeMode: "full-access",
351+
activeTurnId: null,
352+
lastError: null,
353+
updatedAt: NOW,
354+
},
355+
},
356+
};
357+
const checkpointEvent: OrchestrationEvent = {
358+
...sessionEvent,
359+
type: "thread.turn-diff-completed",
360+
sequence: 3,
361+
eventId: EventId.make("checkpoint-finished"),
362+
payload: {
363+
threadId: current.id,
364+
turnId: TurnId.make("turn"),
365+
checkpointTurnCount: 1,
366+
checkpointRef: CheckpointRef.make("checkpoint"),
344367
status: "ready",
345-
providerName: "Codex",
346-
runtimeMode: "full-access",
347-
activeTurnId: null,
348-
lastError: null,
349-
updatedAt: NOW,
368+
files: [],
369+
assistantMessageId: null,
370+
completedAt: NOW,
350371
},
351-
},
352-
});
353-
yield* Queue.take(fixture.reads);
354-
yield* reactor.drain;
355-
expect((yield* Ref.get(fixture.commands))[0]?.branchPullRequest).toEqual(reference(42));
356-
expect((yield* Ref.get(fixture.branchCalls)).filter((call) => call.refresh)).toHaveLength(
357-
1,
358-
);
359-
}).pipe(Effect.provide(fixture.layer));
360-
}),
361-
),
372+
};
373+
const events =
374+
order === "session-first"
375+
? [sessionEvent, checkpointEvent]
376+
: [checkpointEvent, sessionEvent];
377+
for (const event of events) {
378+
yield* fixture.publish(event);
379+
yield* Queue.take(fixture.reads);
380+
yield* reactor.drain;
381+
}
382+
expect((yield* Ref.get(fixture.commands))[0]?.branchPullRequest).toEqual(reference(42));
383+
expect(
384+
(yield* Ref.get(fixture.branchCalls)).filter((call) => call.refresh),
385+
).toHaveLength(1);
386+
}).pipe(Effect.provide(fixture.layer));
387+
}),
388+
),
362389
);
363390

364391
it.effect("uses live worktrees and falls back to the project for removed worktrees", () =>
@@ -455,54 +482,56 @@ describe("ThreadPullRequestReactor", () => {
455482
),
456483
);
457484

458-
it.effect("retries failed settled backfills and stops querying them after success", () =>
459-
Effect.scoped(
460-
Effect.gen(function* () {
461-
const online = yield* Ref.make(false);
462-
const fixture = yield* makeHarness({
463-
threads: [
464-
thread("backfill", { settledOverride: "settled", settledAt: NOW }),
465-
thread("known", {
466-
branch: "known",
467-
settledOverride: "settled",
468-
settledAt: NOW,
469-
branchPullRequest: reference(1),
470-
}),
471-
],
472-
branchPullRequest: ({ cwd }) =>
473-
Ref.get(online).pipe(
474-
Effect.flatMap((connected) =>
475-
connected
476-
? Effect.succeed(branchPullRequest(42, "merged"))
477-
: Effect.fail(
478-
new GitManagerError({
479-
operation: "branchPullRequest",
480-
cwd,
481-
detail: "Offline",
482-
}),
483-
),
485+
it.effect.each(["settled", null] as const)(
486+
"retries settled backfills and stops after success with settledOverride %s",
487+
(settledOverride) =>
488+
Effect.scoped(
489+
Effect.gen(function* () {
490+
const online = yield* Ref.make(false);
491+
const fixture = yield* makeHarness({
492+
threads: [
493+
thread("backfill", { settledOverride, settledAt: NOW }),
494+
thread("known", {
495+
branch: "known",
496+
settledOverride,
497+
settledAt: NOW,
498+
branchPullRequest: reference(1),
499+
}),
500+
],
501+
branchPullRequest: ({ cwd }) =>
502+
Ref.get(online).pipe(
503+
Effect.flatMap((connected) =>
504+
connected
505+
? Effect.succeed(branchPullRequest(42, "merged"))
506+
: Effect.fail(
507+
new GitManagerError({
508+
operation: "branchPullRequest",
509+
cwd,
510+
detail: "Offline",
511+
}),
512+
),
513+
),
484514
),
485-
),
486-
});
487-
yield* Effect.gen(function* () {
488-
const reactor = yield* fixture.start();
489-
expect(yield* Ref.get(fixture.commands)).toHaveLength(0);
490-
yield* Ref.set(online, true);
491-
yield* TestClock.adjust("1 minute");
492-
yield* Queue.take(fixture.reads);
493-
yield* reactor.drain;
494-
expect((yield* Ref.get(fixture.commands))[0]?.threadId).toBe("backfill");
495-
yield* TestClock.adjust("1 minute");
496-
yield* Queue.take(fixture.reads);
497-
yield* reactor.drain;
498-
expect((yield* Ref.get(fixture.branchCalls)).map((call) => call.branch)).toEqual([
499-
"feature",
500-
"feature",
501-
"feature",
502-
]);
503-
}).pipe(Effect.provide(fixture.layer));
504-
}),
505-
),
515+
});
516+
yield* Effect.gen(function* () {
517+
const reactor = yield* fixture.start();
518+
expect(yield* Ref.get(fixture.commands)).toHaveLength(0);
519+
yield* Ref.set(online, true);
520+
yield* TestClock.adjust("1 minute");
521+
yield* Queue.take(fixture.reads);
522+
yield* reactor.drain;
523+
expect((yield* Ref.get(fixture.commands))[0]?.threadId).toBe("backfill");
524+
yield* TestClock.adjust("1 minute");
525+
yield* Queue.take(fixture.reads);
526+
yield* reactor.drain;
527+
expect((yield* Ref.get(fixture.branchCalls)).map((call) => call.branch)).toEqual([
528+
"feature",
529+
"feature",
530+
"feature",
531+
]);
532+
}).pipe(Effect.provide(fixture.layer));
533+
}),
534+
),
506535
);
507536

508537
it.effect("stops retrying a settled backfill after repeated lookup failures", () =>

‎apps/server/src/orchestration/ThreadPullRequestReactor.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@ export const make = Effect.gen(function* () {
101101
const projects = new Map(snapshot.projects.map((project) => [project.id, project]));
102102
if (request.backfill) {
103103
for (const thread of snapshot.threads) {
104-
if (thread.settledOverride === "settled" && thread.branchPullRequest == null) {
104+
if (
105+
(thread.settledOverride === "settled" || thread.settledAt !== null) &&
106+
thread.branchPullRequest == null
107+
) {
105108
pendingBackfill.set(thread.id, BACKFILL_ATTEMPTS);
106109
}
107110
}
@@ -114,7 +117,7 @@ export const make = Effect.gen(function* () {
114117
(thread) =>
115118
thread.archivedAt === null &&
116119
(request.threadId === null || thread.id === request.threadId) &&
117-
(thread.settledOverride !== "settled" ||
120+
((thread.settledOverride !== "settled" && thread.settledAt === null) ||
118121
request.threadId !== null ||
119122
pendingBackfill.has(thread.id)) &&
120123
(thread.branch !== null || thread.branchPullRequest != null),
@@ -327,7 +330,9 @@ export const make = Effect.gen(function* () {
327330
event.payload.session.status !== "running" &&
328331
event.payload.session.status !== "starting"
329332
) {
330-
return worker.enqueue({ threadId: event.payload.threadId, refresh: true });
333+
// Checkpoint completion forces the post-turn read. Session lifecycle
334+
// events reuse it regardless of which event reaches this worker first.
335+
return worker.enqueue({ threadId: event.payload.threadId, refresh: false });
331336
}
332337
break;
333338
case "thread.turn-diff-completed":

0 commit comments

Comments
 (0)