diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 72b3504807c9..ee8605c42b74 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -4962,6 +4962,9 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { expect(NodeFS.readFileSync(NodePath.join(repoDir, "untracked.txt"), "utf8")).toBe( "keep me\n", ); + expect( + (yield* runGit(repoDir, ["config", "branch.feature/pr-safe.t3-pull-request"])).stdout, + ).toBe("https://github.com/pingdotgg/codething-mvp/pull/91\n"); }), ); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index db0d3c91ad19..5f6c469dbf46 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -140,6 +140,12 @@ export class GitManager extends Context.Service< input: { readonly cwd: string; readonly branch: string }, options?: { readonly refresh?: boolean; readonly freshGitState?: boolean }, ) => Effect.Effect; + /** Whether T3 checked this pull request out onto the branch, which ties it to later threads. */ + readonly checkedOutPullRequest: (input: { + readonly cwd: string; + readonly branch: string; + readonly url: string; + }) => Effect.Effect; readonly invalidateLocalStatus: (cwd: string) => Effect.Effect; readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect; readonly invalidateStatus: (cwd: string) => Effect.Effect; @@ -2754,6 +2760,9 @@ export const make = Effect.gen(function* () { repositoryKey: pullRequestRepositoryKey(latest.url), }; }); + + const checkedOutPullRequestKey = (branch: string) => `branch.${branch}.t3-pull-request`; + const branchPullRequest: GitManager["Service"]["branchPullRequest"] = Effect.fn( "branchPullRequest", )(function* ({ cwd, branch }, options) { @@ -2798,6 +2807,16 @@ export const make = Effect.gen(function* () { } return confirmed; }); + + const checkedOutPullRequest: GitManager["Service"]["checkedOutPullRequest"] = Effect.fn( + "checkedOutPullRequest", + )(function* ({ cwd, branch, url }) { + const checkedOutUrl = yield* gitCore + .readConfigValue(cwd, checkedOutPullRequestKey(branch)) + .pipe(Effect.orElseSucceed(() => null)); + return checkedOutUrl === url; + }); + const invalidateLocalStatus: GitManager["Service"]["invalidateLocalStatus"] = Effect.fn( "invalidateLocalStatus", )(function* (cwd) { @@ -3132,7 +3151,25 @@ export const make = Effect.gen(function* () { worktreePath: worktree.worktree.path, isOnPullRequestHead: true, }; - }).pipe(Effect.ensuring(invalidateStatus(input.cwd))); + }).pipe( + Effect.tap((result) => + gitCore + .writeConfigValue({ + cwd: input.cwd, + key: checkedOutPullRequestKey(result.branch), + value: result.pullRequest.url, + }) + .pipe( + Effect.catch((cause) => + Effect.logWarning("GitManager.preparePullRequestThread could not mark the branch", { + branch: result.branch, + cause, + }), + ), + ), + ), + Effect.ensuring(invalidateStatus(input.cwd)), + ); }, asUserAction); const runFeatureBranchStep = Effect.fn("runFeatureBranchStep")(function* ( @@ -3391,6 +3428,7 @@ export const make = Effect.gen(function* () { remoteStatus, status, branchPullRequest, + checkedOutPullRequest, invalidateLocalStatus, invalidateRemoteStatus, invalidateStatus, diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 0c4976dc9059..6764385fb076 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -1156,6 +1156,7 @@ const PullRequestThread = OrchestrationV2AppThreadJsonSchema.mapFields( id, projectId, lineage, + createdAt, branch, worktreePath, archivedAt, @@ -1168,6 +1169,7 @@ const PullRequestThread = OrchestrationV2AppThreadJsonSchema.mapFields( id, projectId, lineage, + createdAt, branch, worktreePath, archivedAt, diff --git a/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts b/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts index 21c54ff4ce1e..0bcdad9c4726 100644 --- a/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadPullRequestService.test.ts @@ -556,6 +556,132 @@ describe("ThreadPullRequestServiceV2 reads", () => { ), ); + it.effect.each([ + { state: "open", checkedOut: false }, + { state: "merged", checkedOut: false }, + { state: "closed", checkedOut: false }, + { state: "merged", checkedOut: true }, + ] as const)( + "associates a $state branch PR (checkedOut=$checkedOut) only with threads that predate its completion or saved it", + ({ state, checkedOut }) => + Effect.scoped( + Effect.gen(function* () { + const reference = { + projectId: ProjectId.make("project-1"), + repository: "acme/project", + number: 42, + url: "https://github.com/acme/project/pull/42", + }; + const terminalAt = "2026-09-19T00:00:00.000Z"; + const detected = { + number: 42, + url: reference.url, + title: "Previous staging work", + baseRef: "main", + headRef: "staging", + state, + updatedAt: terminalAt, + closedAt: state === "closed" ? terminalAt : null, + mergedAt: state === "merged" ? terminalAt : null, + repositoryKey: "github.com/acme/project", + }; + const threads = [ + { ...threadShell("older"), createdAt: DateTime.makeUnsafe("2026-09-18T00:00:00Z") }, + { ...threadShell("same-time"), createdAt: DateTime.makeUnsafe(terminalAt) }, + threadShell("new"), + { ...threadShell("saved"), branchPullRequest: reference }, + { ...threadShell("explicit"), linkedPullRequest: reference }, + ].map((thread) => ({ ...thread, branch: "staging" })); + const project: OrchestrationProjectShell = { + id: reference.projectId, + title: "Project", + workspaceRoot: "/workspace/project", + defaultModelSelection: null, + scripts: [], + createdAt: "2026-08-01T00:00:00.000Z", + updatedAt: "2026-08-01T00:00:00.000Z", + }; + const activation = yield* Deferred.make(); + const sweepStarted = yield* Deferred.make(); + const commands: Array = []; + const dependencies = Layer.mergeAll( + Layer.mock(Orchestrator.OrchestratorV2)({ + streamDomainEvents: Stream.never, + getPullRequestSnapshot: () => + Deferred.succeed(sweepStarted, undefined).pipe( + Effect.as({ snapshotSequence: 1, threads }), + ), + dispatch: (command) => + Effect.sync(() => { + commands.push(command); + return { sequence: 2, storedEvents: [] }; + }), + }), + Layer.mock(ProjectStore.ProjectStoreV2)({ + listShells: () => Effect.succeed([project]), + getShell: () => Effect.succeedSome(project), + }), + Layer.mock(GitManager.GitManager)({ + branchPullRequest: () => Effect.succeed(detected), + checkedOutPullRequest: () => Effect.succeed(checkedOut), + }), + Layer.mock(PullRequestService.PullRequestService)({ + summary: () => + Effect.succeed({ + ...detected, + ...reference, + provider: SourceControlProviderKind.make("github"), + baseBranch: detected.baseRef, + headBranch: detected.headRef, + }), + }), + Layer.mock(RepositoryIdentityResolver.RepositoryIdentityResolver)({ + resolve: () => + Effect.succeed({ + canonicalKey: "github.com/acme/project", + locator: { + source: "git-remote" as const, + remoteName: "origin", + remoteUrl: "git@github.com:acme/project.git", + }, + provider: SourceControlProviderKind.make("github"), + owner: "acme", + name: "project", + }), + }), + Layer.mock(BackgroundPolicy.BackgroundPolicy)({}), + Layer.succeed(ServerActivation.ServerActivation, Deferred.await(activation)), + Layer.succeed( + SecureRandom.SecureRandom, + SecureRandom.make((size) => new Uint8Array(size).fill(1)), + ), + FileSystem.layerNoop({}), + Path.layer, + ); + yield* Effect.gen(function* () { + const service = yield* ThreadPullRequestService.make; + yield* service.start(); + yield* Deferred.succeed(activation, undefined); + yield* Deferred.await(sweepStarted); + yield* service.drain; + }).pipe(Effect.provide(dependencies)); + + expect( + commands.map((command) => { + if (command.type !== "thread.pull-request.sync") throw new Error(command.type); + expect(command).not.toHaveProperty("linkedPullRequest"); + return [command.threadId, command.branchPullRequest]; + }), + ).toEqual( + (state === "open" || checkedOut + ? ["older", "same-time", "new", "explicit"] + : ["older", "same-time"] + ).map((id) => [id, reference]), + ); + }), + ), + ); + it.effect.each([ { state: "closed", sameBranch: false }, { state: "merged", sameBranch: false }, @@ -590,6 +716,7 @@ describe("ThreadPullRequestServiceV2 reads", () => { const activation = yield* Deferred.make(); const sweepStarted = yield* Deferred.make(); const commands: Array = []; + let snapshotThread: unknown; const dependencies = Layer.mergeAll( Layer.mock(Orchestrator.OrchestratorV2)({ streamDomainEvents: Stream.never, @@ -617,6 +744,7 @@ describe("ThreadPullRequestServiceV2 reads", () => { ]) yield* sql.unsafe(`DROP TABLE ${table}`); const snapshot = yield* store.getPullRequestSnapshot(options); + snapshotThread = snapshot.threads[0]; yield* Deferred.succeed(sweepStarted, undefined); return snapshot; }).pipe( @@ -681,6 +809,7 @@ describe("ThreadPullRequestServiceV2 reads", () => { yield* service.drain; }).pipe(Effect.provide(dependencies)); + expect(snapshotThread).toHaveProperty("createdAt", NOW); if (sameBranch) { expect(commands).toEqual([]); } else { diff --git a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts index 4648660ce62b..69fbd1fd918b 100644 --- a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts +++ b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts @@ -3,6 +3,7 @@ import { type OrchestrationProjectShell, type OrchestrationV2DomainEvent, type OrchestrationV2ThreadShell, + type PullRequestSummary, type ThreadId, type ThreadLinkedPullRequest, } from "@t3tools/contracts"; @@ -10,6 +11,7 @@ import { makeDrainableWorker } from "@t3tools/shared/DrainableWorker"; import * as Cause from "effect/Cause"; import * as Context from "effect/Context"; import * as SecureRandom from "effect/crypto/SecureRandom"; +import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; @@ -49,6 +51,19 @@ function samePullRequest( ); } +function pullRequestPredatesThread( + pullRequest: Pick, + createdAt: DateTime.Utc, +): boolean { + const terminalAt = + pullRequest.state === "merged" + ? pullRequest.mergedAt + : pullRequest.state === "closed" + ? pullRequest.closedAt + : null; + return terminalAt != null && Date.parse(terminalAt) < DateTime.toEpochMillis(createdAt); +} + function pullRequestMatchesProject( pullRequest: GitManager.GitBranchPullRequest, project: OrchestrationProjectShell, @@ -209,9 +224,22 @@ export const make = Effect.gen(function* () { } : null; + // Only a new association is refused: a thread keeps a PR it already saved, and a PR the + // user checked out stays with the threads made for it, however long ago it closed. + const isHistoricalFor = (thread: (typeof group)[number]) => + detected !== null && + pullRequestPredatesThread(detected, thread.createdAt) && + !samePullRequest(thread.branchPullRequest, detectedReference); + const dropsHistorical = + detected !== null && + first.branch !== null && + group.some(isHistoricalFor) && + !(yield* git.checkedOutPullRequest({ cwd, branch: first.branch, url: detected.url })); + const plans = yield* Effect.forEach(group, (thread) => Effect.gen(function* () { - let branchPullRequest = detectedReference; + let branchPullRequest = + dropsHistorical && isHistoricalFor(thread) ? null : detectedReference; if ( branchPullRequest === null && thread.branch !== null && diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 9782f258afe1..08b3e58a6a6e 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -58,6 +58,7 @@ import { import { ThreadPullRequestBadgeControl, prStatusIndicator, + resolveDisplayedThreadPr, resolveThreadPullRequestBadge, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; @@ -542,10 +543,10 @@ export function BranchToolbarBranchSelector({ activeThreadBranch, resolvedActiveBranch, }); - const branchPr = - branchPrBranch !== null && branchStatusQuery.data?.refName === branchPrBranch - ? (branchStatusQuery.data.pr ?? null) - : null; + const branchPr = resolveDisplayedThreadPr({ + threadBranch: branchPrBranch, + gitStatus: branchStatusQuery.data, + }); const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId); const linkedStatus = useLinkedThreadPullRequest( environmentId, diff --git a/apps/web/src/components/ThreadStatusIndicators.test.ts b/apps/web/src/components/ThreadStatusIndicators.test.ts index caba939c26e0..ff1c05ad276d 100644 --- a/apps/web/src/components/ThreadStatusIndicators.test.ts +++ b/apps/web/src/components/ThreadStatusIndicators.test.ts @@ -103,6 +103,22 @@ describe("resolveDisplayedThreadPr", () => { expect(resolveDisplayedThreadPrProvider(input)).toEqual(provider); }); + it.each(["open", "merged", "closed"] as const)( + "only shows an unlinked branch PR while open (state=%s)", + (state) => { + const branchPr = { ...mergedPr, state }; + const input = { + threadBranch: featureBranch, + gitStatus: status({ pr: branchPr, sourceControlProvider: provider }), + }; + + expect(resolveDisplayedThreadPr(input)).toEqual(state === "open" ? branchPr : null); + expect(resolveDisplayedThreadPrProvider(input)).toEqual( + state === "open" ? provider : undefined, + ); + }, + ); + it("never attaches a PR reported by main to the feature thread", () => { const mainPr = { number: 99, diff --git a/apps/web/src/components/ThreadStatusIndicators.tsx b/apps/web/src/components/ThreadStatusIndicators.tsx index ea49b01665b6..99e0e01dfe03 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -467,7 +467,7 @@ export function resolveDisplayedThreadPr(input: { threadBranch !== null && gitStatus !== null && gitStatus.refName === threadBranch && - gitStatus.pr != null + gitStatus.pr?.state === "open" ) { return gitStatus.pr; } @@ -481,17 +481,12 @@ export function resolveDisplayedThreadPrProvider(input: { linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; linkedPullRequestStatus?: LinkedThreadPullRequestStatus | null | undefined; }): VcsStatusResult["sourceControlProvider"] | undefined { - const { threadBranch, gitStatus, linkedPullRequest, linkedPullRequestStatus } = input; + const { gitStatus, linkedPullRequest, linkedPullRequestStatus } = input; if (linkedPullRequest != null) { return linkedPullRequestStatus?.sourceControlProvider; } - if ( - threadBranch !== null && - gitStatus !== null && - gitStatus.refName === threadBranch && - gitStatus.pr != null - ) { - return gitStatus.sourceControlProvider; + if (resolveDisplayedThreadPr(input) !== null) { + return gitStatus?.sourceControlProvider; } return undefined;