authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-06-14 11:41:12-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-06-14 11:41:12-07:00
log1db585878ebc85927ddfe8a2bf50764e8e56ae4e
tree939e9a773f952b5c52de1e931513ca98b305feac
parent56e57de9478423becb33723a460eecf508fe1d5d
signature Signed by PGP key B5690EEEBB952194

Guard VCS status updates against stale targets (#3084)

9 files changed, 125 insertions(+), 18 deletions(-)

apps/web/src/components/ChatView.browser.tsx+1
...@@ -100,6 +100,7 @@ vi.mock("../lib/vcsStatusState", () => {...@@ -100,6 +100,7 @@ vi.mock("../lib/vcsStatusState", () => {
100 };100 };
101101
102 return {102 return {
103 getVcsStatusDataForTarget: (state: typeof status) => state.data,
103 getVcsStatusSnapshot: () => status,104 getVcsStatusSnapshot: () => status,
104 useVcsStatus: () => status,105 useVcsStatus: () => status,
105 useVcsStatuses: () => new Map(),106 useVcsStatuses: () => new Map(),
apps/web/src/components/GitActionsControl.browser.tsx+1
...@@ -94,6 +94,7 @@ vi.mock("~/lib/sourceControlActions", () => ({...@@ -94,6 +94,7 @@ vi.mock("~/lib/sourceControlActions", () => ({
94}));94}));
9595
96vi.mock("~/lib/vcsStatusState", () => ({96vi.mock("~/lib/vcsStatusState", () => ({
97 getVcsStatusDataForTarget: (state: { data: unknown }) => state.data,
97 refreshVcsStatus: refreshVcsStatusSpy,98 refreshVcsStatus: refreshVcsStatusSpy,
98 resetVcsStatusStateForTests: () => undefined,99 resetVcsStatusStateForTests: () => undefined,
99 useVcsStatus: vi.fn(() => ({100 useVcsStatus: vi.fn(() => ({
apps/web/src/components/GitActionsControl.logic.test.ts+9
...@@ -1100,6 +1100,15 @@ describe("resolveLiveThreadBranchUpdate", () => {...@@ -1100,6 +1100,15 @@ describe("resolveLiveThreadBranchUpdate", () => {
11001100
1101 assert.equal(update, null);1101 assert.equal(update, null);
1102 });1102 });
1103
1104 it("allows a temporary worktree ref to reconcile to a semantic branch", () => {
1105 const update = resolveLiveThreadBranchUpdate({
1106 threadBranch: "t3code/a9628676",
1107 gitStatus: status({ refName: "feature/diff-panel-toggle" }),
1108 });
1109
1110 assert.deepEqual(update, { branch: "feature/diff-panel-toggle" });
1111 });
1103});1112});
11041113
1105describe("resolveAutoFeatureBranchName", () => {1114describe("resolveAutoFeatureBranchName", () => {
apps/web/src/components/GitActionsControl.tsx+8-5
...@@ -71,7 +71,7 @@ import {...@@ -71,7 +71,7 @@ import {
71 useVcsInitAction,71 useVcsInitAction,
72 useVcsPullAction,72 useVcsPullAction,
73} from "~/lib/sourceControlActions";73} from "~/lib/sourceControlActions";
74import { refreshVcsStatus, useVcsStatus } from "~/lib/vcsStatusState";74import { getVcsStatusDataForTarget, refreshVcsStatus, useVcsStatus } from "~/lib/vcsStatusState";
75import { useSourceControlDiscovery } from "~/lib/sourceControlDiscoveryState";75import { useSourceControlDiscovery } from "~/lib/sourceControlDiscoveryState";
76import { newCommandId, randomUUID } from "~/lib/utils";76import { newCommandId, randomUUID } from "~/lib/utils";
77import { resolvePathLinkTarget } from "~/terminal-links";77import { resolvePathLinkTarget } from "~/terminal-links";
...@@ -1058,10 +1058,13 @@ export default function GitActionsControl({...@@ -1058,10 +1058,13 @@ export default function GitActionsControl({
1058 [persistThreadBranchSync],1058 [persistThreadBranchSync],
1059 );1059 );
10601060
1061 const { data: gitStatus, error: gitStatusError } = useVcsStatus({1061 const vcsStatusTarget = useMemo(
1062 environmentId: activeEnvironmentId,1062 () => ({ environmentId: activeEnvironmentId, cwd: gitCwd }),
1063 cwd: gitCwd,1063 [activeEnvironmentId, gitCwd],
1064 });1064 );
1065 const gitStatusQuery = useVcsStatus(vcsStatusTarget);
1066 const { error: gitStatusError } = gitStatusQuery;
1067 const gitStatus = getVcsStatusDataForTarget(gitStatusQuery, vcsStatusTarget);
1065 const sourceControlPresentation = useMemo(1068 const sourceControlPresentation = useMemo(
1066 () => getSourceControlPresentation(gitStatus?.sourceControlProvider),1069 () => getSourceControlPresentation(gitStatus?.sourceControlProvider),
1067 [gitStatus?.sourceControlProvider],1070 [gitStatus?.sourceControlProvider],
apps/web/src/components/KeybindingsToast.browser.tsx+1
...@@ -67,6 +67,7 @@ vi.mock("../lib/vcsStatusState", () => {...@@ -67,6 +67,7 @@ vi.mock("../lib/vcsStatusState", () => {
67 };67 };
6868
69 return {69 return {
70 getVcsStatusDataForTarget: (state: typeof status) => state.data,
70 getVcsStatusSnapshot: () => status,71 getVcsStatusSnapshot: () => status,
71 useVcsStatus: () => status,72 useVcsStatus: () => status,
72 useVcsStatuses: () => new Map(),73 useVcsStatuses: () => new Map(),
apps/web/src/lib/vcsStatusState.ts+2
...@@ -6,6 +6,7 @@ import {...@@ -6,6 +6,7 @@ import {
6 EMPTY_VCS_STATUS_ATOM,6 EMPTY_VCS_STATUS_ATOM,
7 EMPTY_VCS_STATUS_STATE,7 EMPTY_VCS_STATUS_STATE,
8 createVcsStatusManager,8 createVcsStatusManager,
9 getVcsStatusDataForTarget,
9 getVcsStatusTargetKey,10 getVcsStatusTargetKey,
10 vcsStatusStateAtom,11 vcsStatusStateAtom,
11} from "@t3tools/client-runtime";12} from "@t3tools/client-runtime";
...@@ -19,6 +20,7 @@ import {...@@ -19,6 +20,7 @@ import {
19import { appAtomRegistry } from "../rpc/atomRegistry";20import { appAtomRegistry } from "../rpc/atomRegistry";
2021
21export type { VcsStatusState, VcsStatusTarget };22export type { VcsStatusState, VcsStatusTarget };
23export { getVcsStatusDataForTarget };
2224
23const manager = createVcsStatusManager({25const manager = createVcsStatusManager({
24 getRegistry: () => appAtomRegistry,26 getRegistry: () => appAtomRegistry,
packages/client-runtime/src/gitActions.test.ts created+43
...@@ -0,0 +1,43 @@
1import type { VcsStatusResult } from "@t3tools/contracts";
2import { assert, describe, it } from "vite-plus/test";
3
4import { resolveLiveThreadBranchUpdate } from "./gitActions.js";
5
6function status(refName: string): VcsStatusResult {
7 return {
8 isRepo: true,
9 hasPrimaryRemote: true,
10 isDefaultRef: refName === "main",
11 refName,
12 hasWorkingTreeChanges: false,
13 workingTree: {
14 files: [],
15 insertions: 0,
16 deletions: 0,
17 },
18 hasUpstream: true,
19 aheadCount: 0,
20 behindCount: 0,
21 pr: null,
22 };
23}
24
25describe("resolveLiveThreadBranchUpdate", () => {
26 it("allows a temporary worktree ref to reconcile to a semantic branch", () => {
27 const update = resolveLiveThreadBranchUpdate({
28 threadBranch: "t3code/a9628676",
29 gitStatus: status("feature/diff-panel-toggle"),
30 });
31
32 assert.deepEqual(update, { branch: "feature/diff-panel-toggle" });
33 });
34
35 it("still reconciles ordinary semantic branch changes", () => {
36 const update = resolveLiveThreadBranchUpdate({
37 threadBranch: "feature/old",
38 gitStatus: status("feature/new"),
39 });
40
41 assert.deepEqual(update, { branch: "feature/new" });
42 });
43});
packages/client-runtime/src/vcsStatusState.test.ts+37-4
...@@ -2,7 +2,11 @@ import { EnvironmentId, type VcsStatusResult } from "@t3tools/contracts";...@@ -2,7 +2,11 @@ import { EnvironmentId, type VcsStatusResult } from "@t3tools/contracts";
2import { AtomRegistry } from "effect/unstable/reactivity";2import { AtomRegistry } from "effect/unstable/reactivity";
3import { afterEach, describe, expect, it, vi } from "vite-plus/test";3import { afterEach, describe, expect, it, vi } from "vite-plus/test";
44
5import { type VcsStatusClient, createVcsStatusManager } from "./vcsStatusState.ts";5import {
6 type VcsStatusClient,
7 createVcsStatusManager,
8 getVcsStatusDataForTarget,
9} from "./vcsStatusState.ts";
610
7/* ─── Test helpers ──────────────────────────────────────────────────── */11/* ─── Test helpers ──────────────────────────────────────────────────── */
812
...@@ -57,12 +61,24 @@ function createMockClient(): {...@@ -57,12 +61,24 @@ function createMockClient(): {
57 };61 };
58}62}
5963
60const PENDING = { data: null, error: null, cause: null, isPending: true };
61const EMPTY = { data: null, error: null, cause: null, isPending: false };
62
63const TARGET = { environmentId: EnvironmentId.make("env-local"), cwd: "/repo" } as const;64const TARGET = { environmentId: EnvironmentId.make("env-local"), cwd: "/repo" } as const;
64const FRESH_TARGET = { environmentId: EnvironmentId.make("env-local"), cwd: "/fresh" } as const;65const FRESH_TARGET = { environmentId: EnvironmentId.make("env-local"), cwd: "/fresh" } as const;
65const OTHER_ENV_TARGET = { environmentId: EnvironmentId.make("env-remote"), cwd: "/repo" } as const;66const OTHER_ENV_TARGET = { environmentId: EnvironmentId.make("env-remote"), cwd: "/repo" } as const;
67const TARGET_KEY = "env-local:/repo";
68const PENDING = {
69 targetKey: TARGET_KEY,
70 data: null,
71 error: null,
72 cause: null,
73 isPending: true,
74};
75const EMPTY = {
76 targetKey: null,
77 data: null,
78 error: null,
79 cause: null,
80 isPending: false,
81};
6682
67/* ─── Tests ─────────────────────────────────────────────────────────── */83/* ─── Tests ─────────────────────────────────────────────────────────── */
6884
...@@ -99,6 +115,7 @@ describe("createVcsStatusManager", () => {...@@ -99,6 +115,7 @@ describe("createVcsStatusManager", () => {
99115
100 emit(BASE_STATUS);116 emit(BASE_STATUS);
101 expect(manager.getSnapshot(TARGET)).toEqual({117 expect(manager.getSnapshot(TARGET)).toEqual({
118 targetKey: TARGET_KEY,
102 data: BASE_STATUS,119 data: BASE_STATUS,
103 error: null,120 error: null,
104 cause: null,121 cause: null,
...@@ -130,6 +147,7 @@ describe("createVcsStatusManager", () => {...@@ -130,6 +147,7 @@ describe("createVcsStatusManager", () => {
130147
131 // Snapshot still reflects stream data, not the refresh response148 // Snapshot still reflects stream data, not the refresh response
132 expect(manager.getSnapshot(TARGET)).toEqual({149 expect(manager.getSnapshot(TARGET)).toEqual({
150 targetKey: TARGET_KEY,
133 data: BASE_STATUS,151 data: BASE_STATUS,
134 error: null,152 error: null,
135 cause: null,153 cause: null,
...@@ -160,6 +178,19 @@ describe("createVcsStatusManager", () => {...@@ -160,6 +178,19 @@ describe("createVcsStatusManager", () => {
160 releaseRemote();178 releaseRemote();
161 });179 });
162180
181 it("rejects status data from a previous cwd during target transitions", () => {
182 const staleState = {
183 targetKey: TARGET_KEY,
184 data: BASE_STATUS,
185 error: null,
186 cause: null,
187 isPending: false,
188 };
189
190 expect(getVcsStatusDataForTarget(staleState, FRESH_TARGET)).toBeNull();
191 expect(getVcsStatusDataForTarget(staleState, TARGET)).toBe(BASE_STATUS);
192 });
193
163 it("returns null from refresh when no client is available", async () => {194 it("returns null from refresh when no client is available", async () => {
164 const manager = createVcsStatusManager({195 const manager = createVcsStatusManager({
165 getRegistry: () => atomRegistry,196 getRegistry: () => atomRegistry,
...@@ -204,6 +235,7 @@ describe("createVcsStatusManager", () => {...@@ -204,6 +235,7 @@ describe("createVcsStatusManager", () => {
204235
205 mock.emit(BASE_STATUS);236 mock.emit(BASE_STATUS);
206 expect(manager.getSnapshot(TARGET)).toEqual({237 expect(manager.getSnapshot(TARGET)).toEqual({
238 targetKey: TARGET_KEY,
207 data: BASE_STATUS,239 data: BASE_STATUS,
208 error: null,240 error: null,
209 cause: null,241 cause: null,
...@@ -241,6 +273,7 @@ describe("createVcsStatusManager", () => {...@@ -241,6 +273,7 @@ describe("createVcsStatusManager", () => {
241 for (const listener of connectionListeners) listener();273 for (const listener of connectionListeners) listener();
242274
243 expect(manager.getSnapshot(TARGET)).toEqual({275 expect(manager.getSnapshot(TARGET)).toEqual({
276 targetKey: TARGET_KEY,
244 data: BASE_STATUS,277 data: BASE_STATUS,
245 error: null,278 error: null,
246 cause: null,279 cause: null,
packages/client-runtime/src/vcsStatusState.ts+23-9
...@@ -7,6 +7,7 @@ import type { WsRpcClient } from "./wsRpcClient.ts";...@@ -7,6 +7,7 @@ import type { WsRpcClient } from "./wsRpcClient.ts";
7/* ─── Types ─────────────────────────────────────────────────────────── */7/* ─── Types ─────────────────────────────────────────────────────────── */
88
9export interface VcsStatusState {9export interface VcsStatusState {
10 readonly targetKey: string | null;
10 readonly data: VcsStatusResult | null;11 readonly data: VcsStatusResult | null;
11 readonly error: GitManagerServiceError | null;12 readonly error: GitManagerServiceError | null;
12 readonly cause: Cause.Cause<GitManagerServiceError> | null;13 readonly cause: Cause.Cause<GitManagerServiceError> | null;
...@@ -30,18 +31,22 @@ interface WatchedEntry {...@@ -30,18 +31,22 @@ interface WatchedEntry {
30const NOOP: () => void = () => undefined;31const NOOP: () => void = () => undefined;
3132
32export const EMPTY_VCS_STATUS_STATE = Object.freeze<VcsStatusState>({33export const EMPTY_VCS_STATUS_STATE = Object.freeze<VcsStatusState>({
34 targetKey: null,
33 data: null,35 data: null,
34 error: null,36 error: null,
35 cause: null,37 cause: null,
36 isPending: false,38 isPending: false,
37});39});
3840
39const INITIAL_VCS_STATUS_STATE = Object.freeze<VcsStatusState>({41function initialVcsStatusState(targetKey: string): VcsStatusState {
40 data: null,42 return {
41 error: null,43 targetKey,
42 cause: null,44 data: null,
43 isPending: true,45 error: null,
44});46 cause: null,
47 isPending: true,
48 };
49}
4550
46/* ─── Atoms ─────────────────────────────────────────────────────────── */51/* ─── Atoms ─────────────────────────────────────────────────────────── */
4752
...@@ -49,7 +54,7 @@ const knownVcsStatusKeys = new Set<string>();...@@ -49,7 +54,7 @@ const knownVcsStatusKeys = new Set<string>();
4954
50export const vcsStatusStateAtom = Atom.family((key: string) => {55export const vcsStatusStateAtom = Atom.family((key: string) => {
51 knownVcsStatusKeys.add(key);56 knownVcsStatusKeys.add(key);
52 return Atom.make(INITIAL_VCS_STATUS_STATE).pipe(57 return Atom.make(initialVcsStatusState(key)).pipe(
53 Atom.keepAlive,58 Atom.keepAlive,
54 Atom.withLabel(`vcs-status:${key}`),59 Atom.withLabel(`vcs-status:${key}`),
55 );60 );
...@@ -69,6 +74,14 @@ export function getVcsStatusTargetKey(target: VcsStatusTarget): string | null {...@@ -69,6 +74,14 @@ export function getVcsStatusTargetKey(target: VcsStatusTarget): string | null {
69 return `${target.environmentId}:${target.cwd}`;74 return `${target.environmentId}:${target.cwd}`;
70}75}
7176
77export function getVcsStatusDataForTarget(
78 state: VcsStatusState,
79 target: VcsStatusTarget,
80): VcsStatusResult | null {
81 const targetKey = getVcsStatusTargetKey(target);
82 return targetKey !== null && state.targetKey === targetKey ? state.data : null;
83}
84
72/* ─── Subscription manager ──────────────────────────────────────────── */85/* ─── Subscription manager ──────────────────────────────────────────── */
7386
74export interface VcsStatusManagerConfig {87export interface VcsStatusManagerConfig {
...@@ -107,7 +120,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {...@@ -107,7 +120,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {
107 const current = config.getRegistry().get(atom);120 const current = config.getRegistry().get(atom);
108 const next: VcsStatusState =121 const next: VcsStatusState =
109 current.data === null122 current.data === null
110 ? INITIAL_VCS_STATUS_STATE123 ? initialVcsStatusState(targetKey)
111 : { ...current, error: null, cause: null, isPending: true };124 : { ...current, error: null, cause: null, isPending: true };
112 if (125 if (
113 current.data === next.data &&126 current.data === next.data &&
...@@ -122,6 +135,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {...@@ -122,6 +135,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {
122135
123 function setData(targetKey: string, status: VcsStatusResult): void {136 function setData(targetKey: string, status: VcsStatusResult): void {
124 config.getRegistry().set(vcsStatusStateAtom(targetKey), {137 config.getRegistry().set(vcsStatusStateAtom(targetKey), {
138 targetKey,
125 data: status,139 data: status,
126 error: null,140 error: null,
127 cause: null,141 cause: null,
...@@ -283,7 +297,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {...@@ -283,7 +297,7 @@ export function createVcsStatusManager(config: VcsStatusManagerConfig) {
283 refreshInFlight.clear();297 refreshInFlight.clear();
284 lastRefreshAt.clear();298 lastRefreshAt.clear();
285 for (const key of knownVcsStatusKeys) {299 for (const key of knownVcsStatusKeys) {
286 config.getRegistry().set(vcsStatusStateAtom(key), INITIAL_VCS_STATUS_STATE);300 config.getRegistry().set(vcsStatusStateAtom(key), initialVcsStatusState(key));
287 }301 }
288 knownVcsStatusKeys.clear();302 knownVcsStatusKeys.clear();
289 }303 }