Skip to content
Open
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
31 changes: 31 additions & 0 deletions apps/web/src/components/ChatView.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import type { Thread, TurnDiffSummary } from "../types";
import { makeThreadFixture, makeThreadProjectionFixture } from "../test-fixtures";
import {
agentControlledBrowserCloseConfirmation,
checkoutLandedOnSentDraft,
ENVIRONMENT_RECONNECT_WARNING_GRACE_MS,
getAntigravitySendBlockReason,
resolveBackgroundDraftWorkspaceOptions,
Expand Down Expand Up @@ -2160,3 +2161,33 @@ describe("waitForRevertedMessage", () => {
vi.useRealTimers();
});
});

describe("checkoutLandedOnSentDraft", () => {
const threadId = ThreadId.make("thread-checkout");
const decide = (
overrides: Partial<Parameters<typeof checkoutLandedOnSentDraft>[0]> = {},
): boolean =>
checkoutLandedOnSentDraft({
threadId,
sentDraftThreadIds: new Set(),
draft: { promotedTo: null },
threadShellExists: false,
...overrides,
});

it("defers to the first send while the draft is unsent", () => {
expect(decide()).toBe(false);
expect(decide({ sentDraftThreadIds: new Set([ThreadId.make("another-draft")]) })).toBe(false);
});

it("links now when the draft's send already read it, before its thread shell arrives", () => {
expect(decide({ sentDraftThreadIds: new Set([threadId]) })).toBe(true);
});

it("links now when the draft was promoted or its thread already exists", () => {
expect(
decide({ draft: { promotedTo: { environmentId: EnvironmentId.make("env"), threadId } } }),
).toBe(true);
expect(decide({ draft: null, threadShellExists: true })).toBe(true);
});
});
19 changes: 19 additions & 0 deletions apps/web/src/components/ChatView.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1357,3 +1357,22 @@ export function restorePlanFollowUpComposer(input: {
detectTrigger: true,
});
}

/**
* Whether the draft a pull request checkout landed on had already been sent. That send read the
* draft before the checkout recorded its pull request, so the link is made once the thread
* exists rather than at send. A foreground send leaves no mark on the draft until it is
* finalized, so the caller tracks the draft threads whose send already read the draft.
*/
export function checkoutLandedOnSentDraft(input: {
readonly threadId: ThreadId;
readonly sentDraftThreadIds: ReadonlySet<ThreadId>;
readonly draft: Pick<DraftThreadState, "promotedTo"> | null;
readonly threadShellExists: boolean;
}): boolean {
return (
input.sentDraftThreadIds.has(input.threadId) ||
input.draft?.promotedTo != null ||
input.threadShellExists
);
}
65 changes: 61 additions & 4 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ import {
type Thread,
} from "../types";
import { useTheme } from "../hooks/useTheme";
import { usePullRequestLinking } from "../hooks/usePullRequestLinking";
import { writeTextToClipboard } from "../hooks/useCopyToClipboard";
import { isCommandPaletteOpen } from "../commandPaletteBus";
import { subscribeSnapShotComposerFocus } from "../lib/desktopSnapShot";
Expand Down Expand Up @@ -357,9 +358,11 @@ import {
finalizePromotedDraftThreadByRef,
markPromotedDraftThreadByRef,
restoreFailedBackgroundDraftThread,
checkoutPullRequestUrlToLink,
composerDraftHasUserContent,
type ComposerFileAttachment,
type ComposerImageAttachment,
type DraftCheckoutPullRequest,
type DraftThreadEnvMode,
useComposerDraftStore,
DraftId,
Expand Down Expand Up @@ -414,6 +417,7 @@ import {
useThreadShell,
useThreadRefs,
useThreadVisibleTurnItems,
readThreadShell,
waitForThreadShell,
} from "../state/entities";
import { environmentShell } from "../state/shell";
Expand Down Expand Up @@ -493,6 +497,7 @@ import {
branchMismatchKey,
buildExpiredTerminalContextToastCopy,
buildLocalDraftThread,
checkoutLandedOnSentDraft,
collectUserMessageBlobPreviewUrls,
createLocalDispatchSnapshot,
deriveCommittedServerUserMessageIds,
Expand Down Expand Up @@ -1565,6 +1570,17 @@ export default function ChatView(props: ChatViewProps) {
reportFailure: false,
});
const startThreadTurn = useAtomCommand(threadEnvironment.startTurn, { reportFailure: false });
const pullRequestLinking = usePullRequestLinking(environmentId);
// Links the pull request a checkout prepared to the thread it landed on, the same way the Link
// pull request dialog does. Best effort: the thread already exists, so a failure is only logged.
const linkCheckoutPullRequest = useCallback(
(threadRef: ScopedThreadRef, url: string) => {
pullRequestLinking.changeLink(threadRef, url, true).catch((error: unknown) => {
console.warn("Failed to link the checked-out pull request to its thread.", error);
});
},
[pullRequestLinking],
);
const resumeThreadQueue = useAtomCommand(threadEnvironment.resumeThreadQueue, {
reportFailure: false,
});
Expand Down Expand Up @@ -1927,6 +1943,8 @@ export default function ChatView(props: ChatViewProps) {
const fanoutStateAtom = draftFanoutStateAtom(routeThreadKey);
const fanoutState = useAtomValue(fanoutStateAtom);
const sendInFlightRef = fanoutState.sendInFlight;
// Draft threads whose send already read the draft (see `checkoutLandedOnSentDraft`).
const sentDraftThreadIdsRef = useRef(new Set<ThreadId>());
const [resumingThreadKeys, setResumingThreadKeys] = useState<ReadonlySet<string>>(
() => new Set(),
);
Expand Down Expand Up @@ -2757,7 +2775,12 @@ export default function ChatView(props: ChatViewProps) {
}, []);

const openOrReuseProjectDraftThread = useCallback(
async (input: { branch: string; worktreePath: string | null; envMode: DraftThreadEnvMode }) => {
async (input: {
branch: string;
worktreePath: string | null;
envMode: DraftThreadEnvMode;
checkoutPullRequest: DraftCheckoutPullRequest;
}) => {
if (!activeProject) {
throw new Error("No active project is available for this pull request.");
}
Expand Down Expand Up @@ -2836,14 +2859,33 @@ export default function ChatView(props: ChatViewProps) {
);

const handlePreparedPullRequestThread = useCallback(
async (input: { branch: string; worktreePath: string | null }) => {
await openOrReuseProjectDraftThread({
async (input: { branch: string; worktreePath: string | null; pullRequestUrl: string }) => {
const threadId = await openOrReuseProjectDraftThread({
branch: input.branch,
worktreePath: input.worktreePath,
envMode: input.worktreePath ? "worktree" : "local",
checkoutPullRequest: { url: input.pullRequestUrl, branch: input.branch },
});
// The draft holds the pull request until its first send creates the thread. A draft sent
// while the checkout ran missed it, so its thread is linked here once it exists.
const threadRef = scopeThreadRef(environmentId, threadId);
if (
!checkoutLandedOnSentDraft({
threadId,
sentDraftThreadIds: sentDraftThreadIdsRef.current,
draft: useComposerDraftStore.getState().getDraftSessionByRef(threadRef),
threadShellExists: readThreadShell(threadRef) !== null,
})
) {
return;
}
// A send that fails never creates the thread; the draft keeps the pull request and its
// next send links it.
if (await waitForThreadShell(threadRef)) {
linkCheckoutPullRequest(threadRef, input.pullRequestUrl);
}
},
[openOrReuseProjectDraftThread],
[environmentId, linkCheckoutPullRequest, openOrReuseProjectDraftThread],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);

useEffect(() => {
Expand Down Expand Up @@ -9476,6 +9518,13 @@ export default function ChatView(props: ChatViewProps) {
submissionIntent === "background" && isLocalDraftThread
? scopeThreadRef(environmentId, threadIdForSend)
: null;
// Read before sending: once the thread exists the draft holding it is finalized away.
const checkoutPullRequestUrl = isLocalDraftThread
? checkoutPullRequestUrlToLink(draftThread)
: null;
// Recorded together with the read above, so a checkout finishing on either side of it is
// linked exactly once: by this send, or by the checkout after it.
if (isLocalDraftThread) sentDraftThreadIdsRef.current.add(threadIdForSend);
if (backgroundThreadRef) beginBackgroundDraftSubmissionByRef(backgroundThreadRef);
const startPromise = startThreadTurn({
environmentId,
Expand Down Expand Up @@ -9549,8 +9598,16 @@ export default function ChatView(props: ChatViewProps) {
const startResult = await startPromise;
if (startResult._tag === "Failure") {
failure = startResult;
// The draft is still a draft; its next send reads it again.
sentDraftThreadIdsRef.current.delete(threadIdForSend);
} else {
turnStartSucceeded = true;
if (checkoutPullRequestUrl) {
linkCheckoutPullRequest(
scopeThreadRef(environmentId, threadIdForSend),
checkoutPullRequestUrl,
);
}
// The turn is under way and will spend quota, so that thread's limits
// snapshot is stale. Uploads may have outlasted a navigation, so only
// the sending thread's panel clears.
Expand Down
7 changes: 6 additions & 1 deletion apps/web/src/components/PullRequestThreadDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ interface PullRequestThreadDialogProps {
cwd: string | null;
initialReference: string | null;
onOpenChange: (open: boolean) => void;
onPrepared: (input: { branch: string; worktreePath: string | null }) => Promise<void> | void;
onPrepared: (input: {
branch: string;
worktreePath: string | null;
pullRequestUrl: string;
}) => Promise<void> | void;
}

export function PullRequestThreadDialog({
Expand Down Expand Up @@ -154,6 +158,7 @@ export function PullRequestThreadDialog({
await onPrepared({
branch: result.value.branch,
worktreePath: result.value.worktreePath,
pullRequestUrl: result.value.pullRequest.url,
});
onOpenChange(false);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,18 @@ beforeEach(() => {
.mockResolvedValue({ draftId: newDraftId, threadId: ThreadId.make("new-thread") });
prepareThread.mockReset().mockResolvedValue({
_tag: "Success",
value: { branch: "feature", worktreePath: "/workspace/pr" },
value: {
pullRequest: {
number: 7,
title: "Feature",
url: "https://github.com/acme/widgets/pull/7",
baseBranch: "main",
headBranch: "feature",
state: "open",
},
branch: "feature",
worktreePath: "/workspace/pr",
},
});
});
afterEach(() => {
Expand Down
13 changes: 10 additions & 3 deletions apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1216,10 +1216,10 @@ export function PullRequestDetailPanel({
worktreePath: prepared.value.worktreePath,
envMode: prepared.value.worktreePath === null ? "local" : "worktree",
}).then(
(session) => session !== null,
() => false,
(session) => session,
() => null,
);
if (!pointed) {
if (pointed === null) {
setHandoff(null);
// The checkout is on disk; only the thread failed to move onto it. Writing the task now
// would send the agent at whatever the thread was already open on — which is the one
Expand All @@ -1231,6 +1231,13 @@ export function PullRequestDetailPanel({
});
return;
}
// The draft carries the pull request until its first send makes it a thread, which links it.
useComposerDraftStore.getState().setDraftThreadContext(pointed.draftId, {
checkoutPullRequest: {
url: prepared.value.pullRequest.url,
branch: prepared.value.branch,
},
});
// Released here whatever happened next: a loading toast never expires on its own, so leaving
// this set would spin forever and lock every handoff behind it until a reload.
setHandoff(null);
Expand Down
13 changes: 10 additions & 3 deletions apps/web/src/components/pullRequest/usePullRequestActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,10 @@ export function usePullRequestHandoffs({
worktreePath: prepared.value.worktreePath,
envMode: prepared.value.worktreePath === null ? "local" : "worktree",
}).then(
(session) => session !== null,
() => false,
(session) => session,
() => null,
);
if (!pointed) {
if (pointed === null) {
setHandoff(null);
// The checkout is on disk; only the thread failed to move onto it. Writing the task now
// would send the agent at whatever the thread was already open on — which is the one
Expand All @@ -362,6 +362,13 @@ export function usePullRequestHandoffs({
});
return;
}
// The draft carries the pull request until its first send makes it a thread, which links it.
useComposerDraftStore.getState().setDraftThreadContext(pointed.draftId, {
Comment thread
lnieuwenhuis marked this conversation as resolved.
checkoutPullRequest: {
url: prepared.value.pullRequest.url,
branch: prepared.value.branch,
},
});
// Released here whatever happened next: a loading toast never expires on its own, so leaving
// this set would spin forever and lock every handoff behind it until a reload.
setHandoff(null);
Expand Down
Loading
Loading