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
43 changes: 43 additions & 0 deletions apps/server/src/vcs/VcsStatusBroadcaster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,49 @@ describe("VcsStatusBroadcaster", () => {
},
);

it.effect("publishes a checkout's branch before returning, ahead of the remote read", () => {
const state: Parameters<typeof layerTestFor>[0] = {
currentLocalStatus: baseLocalStatus,
currentRemoteStatus: baseRemoteStatus,
localStatusCalls: 0,
remoteStatusCalls: 0,
localInvalidationCalls: 0,
remoteInvalidationCalls: 0,
};

return Effect.gen(function* () {
const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster;
yield* broadcaster.getStatus({ cwd: "/repo" });
const subscribed = yield* Deferred.make<void>();
const localPublished = yield* Deferred.make<VcsStatusLocalResult>();
const remotePublished = yield* Deferred.make<VcsStatusRemoteResult | null>();
yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe(
Stream.runForEach((event) => {
if (event._tag === "snapshot") return Deferred.succeed(subscribed, undefined);
if (event._tag === "localUpdated") return Deferred.succeed(localPublished, event.local);
return event.remote === null
? Effect.void
: Deferred.succeed(remotePublished, event.remote);
}),
Effect.forkScoped,
);
yield* Deferred.await(subscribed);

const releaseRemote = yield* Deferred.make<void>();
state.beforeRemoteStatus = Deferred.await(releaseRemote);
state.currentLocalStatus = { ...baseLocalStatus, refName: "main" };
state.currentRemoteStatus = remoteStatusWithPr;
yield* broadcaster.refreshAfterCheckout("/repo");

assert.equal((yield* Deferred.await(localPublished)).refName, "main");
assert.isFalse(yield* Deferred.isDone(remotePublished));

yield* Deferred.succeed(releaseRemote, undefined);
assert.deepStrictEqual(yield* Deferred.await(remotePublished), remoteStatusWithPr);
assert.equal((yield* broadcaster.getStatus({ cwd: "/repo" })).pr?.number, 2978);
}).pipe(Effect.provide(layerTestFor(state)), Effect.scoped);
});

it.effect("a remote poll refreshes Changes totals when divergence counts stay the same", () => {
const state: Parameters<typeof layerTestFor>[0] = {
currentLocalStatus: {
Expand Down
62 changes: 46 additions & 16 deletions apps/server/src/vcs/VcsStatusBroadcaster.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as Cause from "effect/Cause";
import * as Context from "effect/Context";
import * as Deferred from "effect/Deferred";
import * as Duration from "effect/Duration";
import * as Effect from "effect/Effect";
import * as Exit from "effect/Exit";
Expand Down Expand Up @@ -211,6 +212,11 @@ export class VcsStatusBroadcaster extends Context.Service<
readonly refreshLocalStatus: (
cwd: string,
) => Effect.Effect<VcsStatusLocalResult, GitManagerServiceError>;
/**
* For a checkout someone is waiting on. Returns once the new branch is published, so the
* caller's reply does not outrun it, and finishes the slower remote read in the background.
*/
readonly refreshAfterCheckout: (cwd: string) => Effect.Effect<void>;
readonly refreshStatus: (
cwd: string,
options?: { readonly localOnly?: boolean },
Expand Down Expand Up @@ -490,6 +496,30 @@ export const make = Effect.gen(function* () {
);
});

const refreshRemoteAndLocalTotals = Effect.fn("VcsStatusBroadcaster.refreshRemoteAndLocalTotals")(
function* (cwd: string, policyCwd: string) {
return yield* withRemoteWriteLock(
cwd,
Effect.gen(function* () {
const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval);
// Bump the PR lookup epoch after earlier remote writers have finished.
yield* workflow.invalidateStatus(cwd);
const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream });
// Fetch can move the base used for local Changes totals.
yield* refreshLocalStatusCore(cwd);
// Without a fetch, the cached behind count cannot justify an automatic pull.
const pulled = refreshUpstream
? yield* maybeAutoPull(cwd, remoteRead, [policyCwd])
: null;
if (pulled !== null) return mergeGitStatusParts(pulled.local, pulled.remote);
yield* updateCachedRemoteStatus(cwd, remoteRead, { publish: true });
const parts = yield* getStatusParts(cwd);
return mergeGitStatusParts(parts.local, parts.remote);
}),
);
},
);

const refreshStatus: VcsStatusBroadcaster["Service"]["refreshStatus"] = Effect.fn(
"VcsStatusBroadcaster.refreshStatus",
)(function* (rawCwd, options) {
Expand All @@ -513,23 +543,22 @@ export const make = Effect.gen(function* () {
}),
);
}
return yield* withRemoteWriteLock(
cwd,
Effect.gen(function* () {
const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval);
// Bump the PR lookup epoch after earlier remote writers have finished.
yield* workflow.invalidateStatus(cwd);
const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream });
// Fetch can move the base used for local Changes totals.
yield* refreshLocalStatusCore(cwd);
// Without a fetch, the cached behind count cannot justify an automatic pull.
const pulled = refreshUpstream ? yield* maybeAutoPull(cwd, remoteRead, [rawCwd]) : null;
if (pulled !== null) return mergeGitStatusParts(pulled.local, pulled.remote);
yield* updateCachedRemoteStatus(cwd, remoteRead, { publish: true });
const parts = yield* getStatusParts(cwd);
return mergeGitStatusParts(parts.local, parts.remote);
}),
return yield* refreshRemoteAndLocalTotals(cwd, rawCwd);
});

const refreshAfterCheckout: VcsStatusBroadcaster["Service"]["refreshAfterCheckout"] = Effect.fn(
"VcsStatusBroadcaster.refreshAfterCheckout",
)(function* (rawCwd) {
const cwd = yield* withFileSystem(normalizeCwd(rawCwd));
const localDone = yield* Deferred.make<void>();
// Owned by the broadcaster so a client disconnecting mid-switch still leaves status correct.
yield* refreshLocalStatusCore(cwd).pipe(
Effect.ensuring(Deferred.succeed(localDone, undefined)),
Effect.andThen(refreshRemoteAndLocalTotals(cwd, rawCwd)),
Effect.ignoreCause({ log: true }),
Effect.forkIn(broadcasterScope),
);
yield* Deferred.await(localDone);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});

const refreshPullRequestStatus: VcsStatusBroadcaster["Service"]["refreshPullRequestStatus"] =
Expand Down Expand Up @@ -762,6 +791,7 @@ export const make = Effect.gen(function* () {
return VcsStatusBroadcaster.of({
getStatus,
refreshLocalStatus,
refreshAfterCheckout,
refreshStatus,
refreshPullRequestStatus,
streamStatus,
Expand Down
14 changes: 12 additions & 2 deletions apps/server/src/ws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3048,9 +3048,19 @@ export const layerWsRpc = (
[WS_METHODS.vcsRemoveWorktree]: (input) =>
gitWorkflow.removeWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
[WS_METHODS.vcsCreateRef]: (input) =>
gitWorkflow.createRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
gitWorkflow
.createRef(input)
.pipe(
Effect.tap(() =>
input.switchRef
? vcsStatusBroadcaster.refreshAfterCheckout(input.cwd)
: refreshGitStatus(input.cwd),
),
),
[WS_METHODS.vcsSwitchRef]: (input) =>
gitWorkflow.switchRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
gitWorkflow
.switchRef(input)
.pipe(Effect.tap(() => vcsStatusBroadcaster.refreshAfterCheckout(input.cwd))),
[WS_METHODS.vcsInit]: (input) =>
vcsProvisioning.initRepository(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))),
[WS_METHODS.reviewGetBranchCommits]: (input) => review.getBranchCommits(input),
Expand Down
29 changes: 28 additions & 1 deletion apps/web/src/components/BranchToolbar.logic.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type VcsRef } from "@t3tools/contracts";
import { EnvironmentId, type VcsRef } from "@t3tools/contracts";
import { describe, expect, it } from "vite-plus/test";
import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git";
import {
Expand All @@ -10,6 +10,7 @@ import {
resolveLocalCheckoutBranchMismatch,
sanitizeNewRefName,
shouldIncludeBranchPickerItem,
isBranchHoldCurrent,
} from "./BranchToolbar.logic";

describe("resolveDraftEnvModeAfterBranchChange", () => {
Expand Down Expand Up @@ -235,6 +236,32 @@ describe("shouldIncludeBranchPickerItem", () => {
});
});

describe("isBranchHoldCurrent", () => {
const environmentId = EnvironmentId.make("env-a");
const hold = { branch: "feature", replaces: "main", environmentId, cwd: "/repo" };
const current = { canonicalBranch: "main", environmentId, cwd: "/repo" };

it("keeps showing the checked-out branch while status still reports the old one", () => {
expect(isBranchHoldCurrent(hold, current)).toBe(true);
});

it("lets go once status moves, even to a different branch than the checkout", () => {
for (const canonicalBranch of ["feature", "other", null]) {
expect(isBranchHoldCurrent(hold, { ...current, canonicalBranch })).toBe(false);
}
});

it("does not apply to another checkout that happens to be on the old branch", () => {
expect(isBranchHoldCurrent(hold, { ...current, cwd: "/other" })).toBe(false);
});

it("does not apply to the same path on another environment", () => {
expect(
isBranchHoldCurrent(hold, { ...current, environmentId: EnvironmentId.make("env-b") }),
).toBe(false);
});
});

describe("sanitizeNewRefName", () => {
it("collapses a run of whitespace into a single dash", () => {
expect(sanitizeNewRefName("new branch")).toBe("new-branch");
Expand Down
26 changes: 26 additions & 0 deletions apps/web/src/components/BranchToolbar.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,32 @@ export function resolveBranchToolbarValue(input: {
return currentGitBranch ?? activeThreadBranch;
}

export interface BranchHold {
readonly branch: string | null;
readonly replaces: string | null;
readonly environmentId: EnvironmentId;
readonly cwd: string;
}

/**
* A finished checkout's branch stays on screen until status reports any change, because the status
* stream and the RPC reply are not ordered and showing status first would flip back to the old branch.
*/
export function isBranchHoldCurrent(
hold: BranchHold,
current: {
readonly canonicalBranch: string | null;
readonly environmentId: EnvironmentId;
readonly cwd: string | null;
},
): boolean {
return (
hold.environmentId === current.environmentId &&
hold.cwd === current.cwd &&
hold.replaces === current.canonicalBranch
);
}

export function resolveBranchTriggerLabel(input: {
activeWorktreePath: string | null;
effectiveEnvMode: EnvMode;
Expand Down
26 changes: 24 additions & 2 deletions apps/web/src/components/BranchToolbarBranchSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ import { parsePullRequestReference } from "@t3tools/client-runtime/pull-request-
import { getSourceControlPresentation } from "../sourceControlPresentation";
import { useComposerMenuProps } from "./chat/composerEventScope";
import {
type BranchHold,
deriveLocalBranchNameFromRemoteRef,
isBranchHoldCurrent,
resolveBranchTriggerLabel,
resolveBranchToolbarPrBranch,
resolveBranchSelectionTarget,
Expand Down Expand Up @@ -286,8 +288,17 @@ export function BranchToolbarBranchSelector({
activeThreadBranch,
currentGitBranch,
});
const [branchHold, setBranchHold] = useState<BranchHold | null>(null);
const branchHoldCurrent =
branchHold !== null &&
isBranchHoldCurrent(branchHold, {
canonicalBranch: canonicalActiveBranch,
environmentId,
cwd: branchCwd,
});
if (branchHold !== null && !branchHoldCurrent) setBranchHold(null);
const [resolvedActiveBranch, setOptimisticBranch] = useOptimistic(
canonicalActiveBranch,
branchHoldCurrent ? branchHold.branch : canonicalActiveBranch,
(_currentBranch: string | null, optimisticBranch: string | null) => optimisticBranch,
);
const listedActiveBranch =
Expand Down Expand Up @@ -376,7 +387,6 @@ export function BranchToolbarBranchSelector({
startBranchActionTransition(async () => {
await action();
branchRefState.refresh();
branchStatusQuery.refresh();
});
};

Expand Down Expand Up @@ -425,6 +435,12 @@ export function BranchToolbarBranchSelector({
? (checkoutResult.value.refName ?? selectedBranchName)
: selectedBranchName;
setOptimisticBranch(nextBranchName);
setBranchHold({
branch: nextBranchName,
replaces: canonicalActiveBranch,
environmentId,
cwd: selectionTarget.checkoutCwd,
});
setThreadBranch(nextBranchName, selectionTarget.nextWorktreePath);
return;
}
Expand Down Expand Up @@ -462,6 +478,12 @@ export function BranchToolbarBranchSelector({
});
if (createBranchResult._tag === "Success") {
setOptimisticBranch(createBranchResult.value.refName);
setBranchHold({
branch: createBranchResult.value.refName,
replaces: canonicalActiveBranch,
environmentId,
cwd: branchCwd,
});
setThreadBranch(createBranchResult.value.refName, activeWorktreePath);
return;
}
Expand Down
Loading