fix(web): retain PR commit provenance across refreshes (#3341)

This commit is contained in:
Carlos Florêncio
2026-09-03 19:54:29 +00:00
committed by GitHub
parent 37054eeb8f
commit a01a00e310
10 changed files with 630 additions and 49 deletions
@@ -1,6 +1,8 @@
import { test, expect } from "../../fixtures/test-base";
import { GitHelper, makeGitEnv } from "../../helpers/git-helper";
import { KanbanPage } from "../../pages/kanban-page";
import { SessionPage } from "../../pages/session-page";
import path from "node:path";
test.describe("PR switcher changes panel", () => {
/**
@@ -18,6 +20,7 @@ test.describe("PR switcher changes panel", () => {
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(120_000);
@@ -45,6 +48,13 @@ test.describe("PR switcher changes panel", () => {
// --- Seed mock GitHub data ---
await apiClient.mockGitHubReset();
await apiClient.mockGitHubSetUser("test-user");
const git = new GitHelper(
path.join(backend.tmpDir, "repos", "e2e-repo"),
makeGitEnv(backend.tmpDir),
);
git.createFile("auth-fix-task.txt", "auth fix task commit");
git.stageFile("auth-fix-task.txt");
const authCommitSHA = git.commit("fix auth token expiry");
// PR #101 files and commits
await apiClient.mockGitHubAddPRs([
@@ -57,6 +67,7 @@ test.describe("PR switcher changes panel", () => {
author_login: "test-user",
repo_owner: "testorg",
repo_name: "testrepo",
head_sha: authCommitSHA,
additions: 35,
deletions: 3,
},
@@ -80,7 +91,7 @@ test.describe("PR switcher changes panel", () => {
]);
await apiClient.mockGitHubAddPRCommits("testorg", "testrepo", 101, [
{
sha: "aaa1111222233334444555566667777aaaabbbb",
sha: authCommitSHA,
message: "fix auth token expiry",
author_login: "test-user",
author_date: "2026-03-01T12:00:00Z",
@@ -162,6 +173,7 @@ test.describe("PR switcher changes panel", () => {
head_branch: "main",
base_branch: "main",
author_login: "test-user",
head_sha: authCommitSHA,
additions: 35,
deletions: 3,
});
@@ -198,6 +210,13 @@ test.describe("PR switcher changes panel", () => {
await expect(session.commitsSection()).toBeVisible();
await expect(session.commitsSection().getByText("fix auth token expiry")).toBeVisible();
const authCommitRow = session
.commitsSection()
.getByTestId(`commit-row-${authCommitSHA.slice(0, 7)}`);
await expect(authCommitRow.getByTestId("commit-provenance")).toHaveAttribute(
"data-commit-provenance",
"pushed",
);
// --- Switch to Task B ---
await session.taskInSidebar("Dashboard Task").click();
@@ -236,6 +255,23 @@ test.describe("PR switcher changes panel", () => {
await expect(session.changes.getByText("add api client")).not.toBeVisible();
// --- Switch back to Task A to confirm data reappears ---
// Force the newly versioned provider request to fail. The last confirmed
// same-PR provenance must remain visible while the selected task refreshes.
await apiClient.mockGitHubSetPRCommitsFailures("testorg", "testrepo", 101, 100);
await apiClient.mockGitHubAssociateTaskPR({
task_id: taskA.id,
owner: "testorg",
repo: "testrepo",
pr_number: 101,
pr_url: "https://github.com/testorg/testrepo/pull/101",
pr_title: "Fix auth bug",
head_branch: "main",
base_branch: "main",
author_login: "test-user",
head_sha: authCommitSHA,
additions: 35,
deletions: 3,
});
await session.taskInSidebar("Auth Fix Task").click();
await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), {
timeout: 15_000,
@@ -248,5 +284,9 @@ test.describe("PR switcher changes panel", () => {
await expect(session.prFilesSection().getByText("auth.go")).toBeVisible();
await expect(session.prFilesSection().getByText("auth_test.go")).toBeVisible();
await expect(session.commitsSection().getByText("fix auth token expiry")).toBeVisible();
await expect(authCommitRow.getByTestId("commit-provenance")).toHaveAttribute(
"data-commit-provenance",
"pushed",
);
});
});
@@ -4,6 +4,7 @@ import { t } from "@/lib/i18n";
export type PRCommitsState = {
commits: PRCommitInfo[];
authoritativeCommits: PRCommitInfo[];
providerHead: string | null;
providerCommitsComplete: boolean;
loading: boolean;
@@ -41,6 +42,7 @@ type ResourceEntry = {
identity: string;
request: PRCommitsRequest | null;
snapshot: PRCommitsState;
successfulCommits: PRCommitInfo[];
loaded: boolean;
promise: Promise<PRCommitsState> | null;
listeners: Set<() => void>;
@@ -53,6 +55,7 @@ const DEFAULT_RETRY_DELAY_MS = 100;
const DEFAULT_MAX_ENTRIES = 24;
const EMPTY_STATE: PRCommitsState = {
commits: [],
authoritativeCommits: [],
providerHead: null,
providerCommitsComplete: false,
loading: false,
@@ -63,6 +66,10 @@ const PENDING_STATE: PRCommitsState = {
loading: true,
};
function pendingState(commits: PRCommitInfo[] = []): PRCommitsState {
return { ...PENDING_STATE, commits };
}
function identityFor(request: PRCommitsRequest): string {
return [request.workspaceId, request.owner, request.repo, request.prNumber].join("\0");
}
@@ -83,8 +90,10 @@ async function requestPRCommits(request: PRCommitsRequest): Promise<PRCommitsRes
}
function successState(response: PRCommitsResponse | null): PRCommitsState {
const commits = response?.commits ?? [];
return {
commits: response?.commits ?? [],
commits,
authoritativeCommits: commits,
providerHead: response?.head_sha ?? null,
providerCommitsComplete: response?.complete === true,
loading: false,
@@ -92,19 +101,25 @@ function successState(response: PRCommitsResponse | null): PRCommitsState {
};
}
function failedState(error: unknown): PRCommitsState {
function failedState(error: unknown, commits: PRCommitInfo[] = []): PRCommitsState {
return {
...EMPTY_STATE,
commits,
error: errorMessage(error),
};
}
function createEntry(request: PRCommitsRequest | null, key: string): ResourceEntry {
function createEntry(
request: PRCommitsRequest | null,
key: string,
successfulCommits: PRCommitInfo[] = [],
): ResourceEntry {
return {
key,
identity: request ? identityFor(request) : key,
request,
snapshot: PENDING_STATE,
snapshot: pendingState(successfulCommits),
successfulCommits,
loaded: false,
promise: null,
listeners: new Set(),
@@ -121,6 +136,7 @@ function createEntry(request: PRCommitsRequest | null, key: string): ResourceEnt
*/
type PRCommitsResourceStore = {
entries: Map<string, ResourceEntry>;
currentKeyByIdentity: Map<string, string>;
retryDelayMs: number;
maxEntries: number;
usageCounter: number;
@@ -130,6 +146,13 @@ function touch(store: PRCommitsResourceStore, entry: ResourceEntry): void {
entry.lastUsed = ++store.usageCounter;
}
function deleteEntry(store: PRCommitsResourceStore, entry: ResourceEntry): void {
store.entries.delete(entry.key);
if (store.currentKeyByIdentity.get(entry.identity) === entry.key) {
store.currentKeyByIdentity.delete(entry.identity);
}
}
function entryFor(
store: PRCommitsResourceStore,
key: string,
@@ -140,25 +163,32 @@ function entryFor(
if (request) {
existing.request = request;
existing.identity = identityFor(request);
store.currentKeyByIdentity.set(existing.identity, key);
}
touch(store, existing);
return existing;
}
const entry = createEntry(request, key);
const identity = request ? identityFor(request) : key;
const retainedKey = store.currentKeyByIdentity.get(identity);
const retainedEntry = retainedKey ? store.entries.get(retainedKey) : undefined;
const entry = createEntry(request, key, retainedEntry?.successfulCommits ?? []);
store.entries.set(key, entry);
if (request) store.currentKeyByIdentity.set(identity, key);
touch(store, entry);
return entry;
}
function prune(store: PRCommitsResourceStore, entry: ResourceEntry): void {
for (const [key, candidate] of store.entries) {
if (
key !== entry.key &&
candidate.identity === entry.identity &&
!candidate.promise &&
candidate.listeners.size === 0
) {
store.entries.delete(key);
if (store.currentKeyByIdentity.get(entry.identity) === entry.key) {
for (const candidate of store.entries.values()) {
if (
candidate !== entry &&
candidate.identity === entry.identity &&
!candidate.promise &&
candidate.listeners.size === 0
) {
deleteEntry(store, candidate);
}
}
}
@@ -169,7 +199,7 @@ function prune(store: PRCommitsResourceStore, entry: ResourceEntry): void {
.sort((left, right) => left.lastUsed - right.lastUsed);
for (const candidate of candidates) {
if (store.entries.size <= store.maxEntries) break;
store.entries.delete(candidate.key);
deleteEntry(store, candidate);
}
}
@@ -206,6 +236,7 @@ function publish(
): void {
if (!isCurrent(store, entry)) return;
entry.snapshot = snapshot;
if (loaded && !snapshot.error) entry.successfulCommits = snapshot.authoritativeCommits;
entry.loaded = loaded;
touch(store, entry);
notify(entry);
@@ -216,9 +247,16 @@ function publishFailure(
entry: ResourceEntry,
error: unknown,
): PRCommitsState {
const snapshot = failedState(error);
const snapshot = failedState(error, entry.successfulCommits);
publish(store, entry, snapshot, false);
if (isCurrent(store, entry) && entry.listeners.size === 0) store.entries.delete(entry.key);
if (
isCurrent(store, entry) &&
entry.listeners.size === 0 &&
(store.currentKeyByIdentity.get(entry.identity) !== entry.key ||
entry.successfulCommits.length === 0)
) {
deleteEntry(store, entry);
}
return snapshot;
}
@@ -229,7 +267,11 @@ async function attempt(
{ response: PRCommitsResponse | null; error: null } | { response: null; error: unknown }
> {
try {
return { response: await requester(request), error: null };
const response = await requester(request);
if (response === null) {
return { response: null, error: new Error(t("github:failedToFetchPrCommits")) };
}
return { response, error: null };
} catch (error) {
return { response: null, error };
}
@@ -250,7 +292,9 @@ async function run(
}
if (isCurrent(store, entry) && !entry.orphaned) await waitBeforeRetry(store, entry);
if (!isCurrent(store, entry) || entry.orphaned) return failedState(first.error);
if (!isCurrent(store, entry) || entry.orphaned) {
return failedState(first.error, entry.successfulCommits);
}
const second = await attempt(requester, request);
if (!second.error) {
@@ -275,7 +319,7 @@ function load(
if (!force && entry.loaded && !entry.snapshot.error) return Promise.resolve(entry.snapshot);
entry.loaded = false;
entry.snapshot = { ...PENDING_STATE };
entry.snapshot = pendingState(entry.successfulCommits);
notify(entry);
const promise = run(store, requester, entry, request);
entry.promise = promise;
@@ -290,9 +334,13 @@ function load(
return promise;
}
function subscribe(store: PRCommitsResourceStore, key: string, listener: () => void): () => void {
if (!key) return () => undefined;
const entry = entryFor(store, key);
function subscribe(
store: PRCommitsResourceStore,
request: PRCommitsRequest | null,
listener: () => void,
): () => void {
if (!request) return () => undefined;
const entry = entryFor(store, request.sourceKey, request);
entry.listeners.add(listener);
entry.orphaned = false;
touch(store, entry);
@@ -301,14 +349,23 @@ function subscribe(store: PRCommitsResourceStore, key: string, listener: () => v
if (entry.listeners.size > 0) return;
entry.orphaned = Boolean(entry.promise);
clearRetryWait(entry);
if (!entry.promise && entry.snapshot.error) store.entries.delete(entry.key);
if (
!entry.promise &&
entry.snapshot.error &&
(store.currentKeyByIdentity.get(entry.identity) !== entry.key ||
entry.successfulCommits.length === 0)
) {
deleteEntry(store, entry);
}
};
}
function getSnapshot(store: PRCommitsResourceStore, key: string): PRCommitsState {
if (!key) return EMPTY_STATE;
const entry = store.entries.get(key);
if (!entry) return PENDING_STATE;
function getSnapshot(
store: PRCommitsResourceStore,
request: PRCommitsRequest | null,
): PRCommitsState {
if (!request) return EMPTY_STATE;
const entry = entryFor(store, request.sourceKey, request);
// useSyncExternalStore reads during render; touching here keeps actively
// rendered entries recent for bounded LRU eviction.
touch(store, entry);
@@ -321,14 +378,16 @@ export function createPRCommitsResource(
) {
const store: PRCommitsResourceStore = {
entries: new Map(),
currentKeyByIdentity: new Map(),
retryDelayMs: Math.max(0, options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS),
maxEntries: Math.max(1, options.maxEntries ?? DEFAULT_MAX_ENTRIES),
usageCounter: 0,
};
return {
getSnapshot: (key: string) => getSnapshot(store, key),
getSnapshot: (request: PRCommitsRequest | null) => getSnapshot(store, request),
load: (request: PRCommitsRequest, force = false) => load(store, requester, request, force),
subscribe: (key: string, listener: () => void) => subscribe(store, key, listener),
subscribe: (request: PRCommitsRequest | null, listener: () => void) =>
subscribe(store, request, listener),
};
}
@@ -12,6 +12,7 @@ let websocketClient: { request: typeof requestMock } | null = { request: request
const SHARED_COMMIT_SHA = "shared-sha";
const RETRY_COMMIT_SHA = "retry-sha";
const REFRESHED_COMMIT_SHA = "refreshed-sha";
const STABLE_COMMIT_SHA = "stable-sha";
vi.mock("@/lib/ws/connection", () => ({
getWebSocketClient: () => websocketClient,
}));
@@ -115,7 +116,7 @@ describe("usePRCommits request ownership", () => {
});
const resource = createPRCommitsResource(requester, { retryDelayMs: 10 });
const request = resourceRequest("final-error");
const unsubscribe = resource.subscribe(request.sourceKey, vi.fn());
const unsubscribe = resource.subscribe(request, vi.fn());
const failed = resource.load(request);
await Promise.resolve();
@@ -126,7 +127,7 @@ describe("usePRCommits request ownership", () => {
providerCommitsComplete: false,
error: "final provider failure",
});
expect(resource.getSnapshot(request.sourceKey).error).toBe("final provider failure");
expect(resource.getSnapshot(request).error).toBe("final provider failure");
const recovered = resource.load(request);
await expect(recovered).resolves.toMatchObject({ providerHead: "recovered-sha" });
@@ -181,6 +182,137 @@ describe("usePRCommits manual refresh", () => {
});
});
describe("usePRCommits retained evidence", () => {
// @covers AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7
it("retains resolved display commits while a same-PR sync-version refresh is pending", async () => {
const refresh = deferred<{ commits: PRCommitInfo[]; head_sha: string; complete: boolean }>();
requestMock
.mockResolvedValueOnce({
commits: [commit(STABLE_COMMIT_SHA)],
head_sha: STABLE_COMMIT_SHA,
complete: true,
})
.mockReturnValueOnce(refresh.promise);
const { result, rerender } = renderHook(
({ syncVersion }) => usePRCommits("acme", "app", 1, syncVersion),
{ initialProps: { syncVersion: "retention-old-sync" }, wrapper },
);
await waitFor(() => expect(result.current.commits[0]?.sha).toBe(STABLE_COMMIT_SHA));
rerender({ syncVersion: "retention-new-sync" });
await waitFor(() => expect(requestMock).toHaveBeenCalledTimes(2));
expect(result.current.commits[0]?.sha).toBe(STABLE_COMMIT_SHA);
expect(result.current.authoritativeCommits).toEqual([]);
expect(result.current.loading).toBe(true);
await act(async () => {
refresh.resolve({
commits: [commit(STABLE_COMMIT_SHA)],
head_sha: STABLE_COMMIT_SHA,
complete: true,
});
await refresh.promise;
});
});
it("retains resolved display commits when a same-PR refresh fails", async () => {
const requester = vi
.fn()
.mockResolvedValueOnce({
commits: [commit(STABLE_COMMIT_SHA)],
head_sha: STABLE_COMMIT_SHA,
complete: true,
})
.mockRejectedValueOnce(new Error("temporary refresh failure"))
.mockRejectedValueOnce(new Error("final refresh failure"));
const resource = createPRCommitsResource(requester, { retryDelayMs: 0 });
await resource.load(resourceRequest("failure-old-version"));
const failed = await resource.load(resourceRequest("failure-new-version"));
expect(failed).toMatchObject({
commits: [commit(STABLE_COMMIT_SHA)],
authoritativeCommits: [],
providerHead: null,
providerCommitsComplete: false,
loading: false,
error: "final refresh failure",
});
});
it("replaces retained display commits after a same-PR refresh succeeds", async () => {
requestMock
.mockResolvedValueOnce({
commits: [commit(STABLE_COMMIT_SHA)],
head_sha: STABLE_COMMIT_SHA,
complete: true,
})
.mockResolvedValueOnce({
commits: [commit(REFRESHED_COMMIT_SHA)],
head_sha: REFRESHED_COMMIT_SHA,
complete: true,
});
const { result, rerender } = renderHook(
({ syncVersion }) => usePRCommits("acme", "app", 1, syncVersion),
{ initialProps: { syncVersion: "success-old-version" }, wrapper },
);
await waitFor(() => expect(result.current.commits[0]?.sha).toBe(STABLE_COMMIT_SHA));
rerender({ syncVersion: "success-new-version" });
await waitFor(() => expect(result.current.commits[0]?.sha).toBe(REFRESHED_COMMIT_SHA));
expect(result.current.authoritativeCommits[0]?.sha).toBe(REFRESHED_COMMIT_SHA);
expect(result.current.loading).toBe(false);
});
it("does not retain commits across different PR identities", async () => {
const pending = deferred<{ commits: PRCommitInfo[] }>();
const requester = vi
.fn()
.mockResolvedValueOnce({
commits: [commit("first-pr-sha")],
head_sha: "first-pr-sha",
complete: true,
})
.mockReturnValueOnce(pending.promise);
const resource = createPRCommitsResource(requester);
const secondPR = resourceRequest("second-pr-version", 2);
await resource.load(resourceRequest("first-pr-version", 1));
const secondLoad = resource.load(secondPR);
expect(resource.getSnapshot(secondPR).commits).toEqual([]);
pending.resolve({ commits: [] });
await secondLoad;
});
});
describe("usePRCommits unavailable client", () => {
it("retains resolved display commits when the WebSocket client is unavailable", async () => {
requestMock.mockResolvedValueOnce({
commits: [commit(STABLE_COMMIT_SHA)],
head_sha: STABLE_COMMIT_SHA,
complete: true,
});
const resource = createPRCommitsResource(undefined, { retryDelayMs: 0 });
const initialRequest = resourceRequest("null-old-version");
const refreshedRequest = resourceRequest("null-new-version");
await resource.load(initialRequest);
websocketClient = null;
const failed = await resource.load(refreshedRequest);
expect(failed.commits).toEqual([commit(STABLE_COMMIT_SHA)]);
expect(failed.authoritativeCommits).toEqual([]);
expect(failed.error).not.toBeNull();
expect(requestMock).toHaveBeenCalledTimes(1);
});
});
describe("usePRCommits stale results and eviction", () => {
it("does not let an older sync-version response replace the active result", async () => {
const first = deferred<{ commits: PRCommitInfo[]; head_sha: string; complete: boolean }>();
@@ -208,6 +340,34 @@ describe("usePRCommits stale results and eviction", () => {
expect(result.current.providerHead).toBe("new-sha");
});
it("retains the latest selected version after an older request resolves last", async () => {
const first = deferred<{ commits: PRCommitInfo[]; head_sha: string; complete: boolean }>();
const second = deferred<{ commits: PRCommitInfo[]; head_sha: string; complete: boolean }>();
const third = deferred<{ commits: PRCommitInfo[]; head_sha: string; complete: boolean }>();
const requester = vi
.fn()
.mockReturnValueOnce(first.promise)
.mockReturnValueOnce(second.promise)
.mockReturnValueOnce(third.promise);
const resource = createPRCommitsResource(requester);
const firstRequest = resourceRequest("retained-old-version");
const secondRequest = resourceRequest("retained-current-version");
const thirdRequest = resourceRequest("retained-next-version");
const firstLoad = resource.load(firstRequest);
const secondLoad = resource.load(secondRequest);
second.resolve({ commits: [commit("new-sha")], head_sha: "new-sha", complete: true });
await secondLoad;
first.resolve({ commits: [commit("old-sha")], head_sha: "old-sha", complete: true });
await firstLoad;
const thirdLoad = resource.load(thirdRequest);
expect(resource.getSnapshot(thirdRequest).commits[0]?.sha).toBe("new-sha");
third.resolve({ commits: [commit("third-sha")], head_sha: "third-sha", complete: true });
await thirdLoad;
});
it("evicts superseded and old cache entries", async () => {
const requester = vi.fn().mockImplementation(async (request: PRCommitsRequest) => ({
commits: [commit(request.sourceKey)],
@@ -235,6 +395,7 @@ describe("usePRCommits hook view", () => {
const staleState: KeyedPRCommitsState = {
sourceKey: "workspace-1/acme/app/1/old-refresh",
commits: [commit("first-sha")],
authoritativeCommits: [commit("first-sha")],
providerHead: "first-sha",
providerCommitsComplete: true,
loading: false,
@@ -243,6 +404,7 @@ describe("usePRCommits hook view", () => {
expect(resolvePRCommitsView(staleState, "workspace-1/acme/other-app/2/new-refresh")).toEqual({
commits: [],
authoritativeCommits: [],
providerHead: null,
providerCommitsComplete: false,
loading: true,
+26 -11
View File
@@ -1,6 +1,6 @@
"use client";
import { useCallback, useEffect, useRef, useSyncExternalStore } from "react";
import { useCallback, useEffect, useMemo, useRef, useSyncExternalStore } from "react";
import { useAppStore } from "@/components/state-provider";
import { prCommitsResource, type PRCommitsState } from "./pr-commits-resource";
@@ -13,6 +13,7 @@ export function resolvePRCommitsView(
if (state.sourceKey === requestedKey) {
return {
commits: state.commits,
authoritativeCommits: state.authoritativeCommits,
providerHead: state.providerHead,
providerCommitsComplete: state.providerCommitsComplete,
loading: state.loading,
@@ -21,6 +22,7 @@ export function resolvePRCommitsView(
}
return {
commits: [],
authoritativeCommits: [],
providerHead: null,
providerCommitsComplete: false,
loading: requestedKey !== "",
@@ -43,25 +45,38 @@ export function usePRCommits(
const sourceKey = hasParams
? `${workspaceId}/${owner}/${repo}/${prNumber}/${refreshKey ?? ""}`
: "";
const subscribe = useCallback(
(listener: () => void) => prCommitsResource.subscribe(sourceKey, listener),
[sourceKey],
const request = useMemo(
() =>
hasParams
? {
workspaceId,
owner,
repo,
prNumber,
sourceKey,
}
: null,
[hasParams, workspaceId, owner, repo, prNumber, sourceKey],
);
const getSnapshot = useCallback(() => prCommitsResource.getSnapshot(sourceKey), [sourceKey]);
const subscribe = useCallback(
(listener: () => void) => prCommitsResource.subscribe(request, listener),
[request],
);
const getSnapshot = useCallback(() => prCommitsResource.getSnapshot(request), [request]);
const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot);
const paramsKeyRef = useRef<string>("");
const refresh = useCallback(async () => {
if (!workspaceId || !owner || !repo || !prNumber) return null;
return prCommitsResource.load({ workspaceId, owner, repo, prNumber, sourceKey }, true);
}, [workspaceId, owner, repo, prNumber, sourceKey]);
if (!request) return null;
return prCommitsResource.load(request, true);
}, [request]);
useEffect(() => {
if (sourceKey === paramsKeyRef.current) return;
paramsKeyRef.current = sourceKey;
if (!workspaceId || !owner || !repo || !prNumber) return;
void prCommitsResource.load({ workspaceId, owner, repo, prNumber, sourceKey });
}, [workspaceId, owner, repo, prNumber, sourceKey]);
if (!request) return;
void prCommitsResource.load(request);
}, [request, sourceKey]);
return { ...resolvePRCommitsView({ sourceKey, ...snapshot }, sourceKey), refresh };
}
@@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
import type { PRCommitInfo, TaskPR } from "@/lib/types/github";
import type { GitStatusEntry } from "@/lib/state/slices/session-runtime/types";
import { useRemoteContributionRelation } from "./use-remote-contribution-relation";
import { remoteContributionActionPolicy } from "./remote-contribution-relation";
const mocks = vi.hoisted(() => ({
statuses: [] as Array<{ repository_name: string; status: GitStatusEntry }>,
@@ -10,6 +11,9 @@ const mocks = vi.hoisted(() => ({
selectedPR: null as TaskPR | null,
selectedKey: null as string | null,
providerHead: "provider-head",
authoritativeCommits: [{ sha: "provider-head" }] as PRCommitInfo[],
loading: false,
error: null as string | null,
repositoryName: "frontend",
}));
@@ -29,10 +33,11 @@ vi.mock("@/hooks/domains/github/use-review-pr-selection", () => ({
vi.mock("@/hooks/domains/github/use-pr-commits", () => ({
usePRCommits: () => ({
commits: [{ sha: mocks.providerHead }] as PRCommitInfo[],
authoritativeCommits: mocks.authoritativeCommits,
providerHead: mocks.providerHead,
providerCommitsComplete: true,
loading: false,
error: null,
loading: mocks.loading,
error: mocks.error,
refresh: vi.fn(),
}),
}));
@@ -125,6 +130,9 @@ describe("useRemoteContributionRelation repository scoping", () => {
mocks.selectedKey = null;
mocks.statuses = [];
mocks.repositoryName = "frontend";
mocks.authoritativeCommits = [{ sha: mocks.providerHead }] as PRCommitInfo[];
mocks.loading = false;
mocks.error = null;
});
it.each([
@@ -167,6 +175,34 @@ describe("useRemoteContributionRelation repository scoping", () => {
});
});
// @covers AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7
it("keeps retained commits visible without authorizing actions during refresh", () => {
mocks.authoritativeCommits = [];
mocks.loading = true;
mocks.statuses = [
status("", mocks.providerHead, "provider-base", {
remote_ahead: 1,
remote_behind: 0,
}),
];
const { result } = renderHook(() => useRemoteContributionRelation("session-1"));
expect(result.current.commits).toEqual([{ sha: mocks.providerHead }]);
expect(result.current.relation).toMatchObject({
kind: "unknown",
action: "unavailable_evidence",
canReplaceRemote: false,
canUseRemote: false,
});
expect(remoteContributionActionPolicy(result.current.relation)).toMatchObject({
pushDisabled: true,
pullDisabled: true,
replaceDisabled: true,
useDisabled: true,
});
});
it("ignores a merged Review PR when a newer PR matches the checked-out branch", () => {
const historicalPR = taskPR({
id: "pr-historical",
@@ -64,7 +64,7 @@ export function useRemoteContributionRelation(
() =>
classifyRemoteContribution({
hasSelectedPR: Boolean(selectedPR),
providerCommits: commitsState.commits,
providerCommits: commitsState.authoritativeCommits,
providerHead: commitsState.providerHead,
providerCommitsComplete: commitsState.providerCommitsComplete,
providerLoading: commitsState.loading,
@@ -78,7 +78,7 @@ export function useRemoteContributionRelation(
}),
[
selectedPR,
commitsState.commits,
commitsState.authoritativeCommits,
commitsState.providerHead,
commitsState.providerCommitsComplete,
commitsState.loading,
@@ -0,0 +1,136 @@
---
created: 2026-09-03
status: completed
requirements:
- REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001
system_design:
- ../../specs/tasks/system-design/remote-contribution-tasks.md
legacy_specs: []
---
# Implementation Plan: Commit Provenance Refresh Stability
## Overview
Keep confirmed pull-request commit provenance stable while the provider commit resource refreshes for
the same contribution. Implement the correction in one vertical work order because the cache contract,
safe relation classification, Changes projection, and regression evidence are one coupled result.
## Root cause
`usePRCommits` keys provider evidence by workspace, repository, pull request, and
`last_synced_at`. When that version changes, `pr-commits-resource.ts` creates a pending entry whose
commit list is empty and prunes the previous version. `useRemoteContributionRelation` passes that empty
list to the Changes projection. `mergeCommits` can then recognize only local Git history, so commits
whose provider membership was already confirmed temporarily render with the emerald unpushed marker.
The accepted provider response restores the SHA matches and the neutral pushed marker.
Task switching exposes this transition because task restoration resubscribes Changes consumers and can
refresh the task pull-request snapshot. The supplied trace shows repeated `session.git.commits` reads.
The later Git status update reports `changed=false`. Thus, provider-enrichment state causes the marker
transition. A checkout change does not cause it.
## Scope
### In scope
- Retain the most recent successful provider commit list across evidence-version changes for the same
workspace, repository, and pull request.
- Keep retained commits display-only while the current version is pending or failed.
- After a successful current-version response, replace the retained provenance.
- Preserve identity isolation, stale-result rejection, retry behavior, deduplication, and bounded cache
eviction.
- Add a fail-before-fix hook regression and task-switch browser evidence.
### Out of scope
- Persist provider snapshots across page reloads or application restarts.
- Change local Git ahead/behind computation, push routing, commit marker styling, or copy.
- Change Changes-panel layout, touch behavior, scrolling, navigation, or mobile composition.
- Reduce the duplicate Git-status subscriptions visible in the supplied trace. They are diagnostic
noise and do not cause the incorrect marker state.
- Treat retained provider commits as current evidence for Push, Pull, or contribution replacement.
## Technical approach
### Provider commit resource
`PRCommitsState` and `apps/web/hooks/domains/github/pr-commits-resource.ts` separate the current
authoritative commit snapshot from the last successful display snapshot. When a new `sourceKey` version
starts, use the existing `identityFor` key to find retained display evidence. This key contains the
`workspaceId`, owner, repository, and pull request number. Do not retain across a different identity.
Pending and failed current versions expose retained commits only through the display projection. Their
authoritative commits, provider head, and completeness remain empty or false, with the current loading
or error state intact. Successful current evidence atomically replaces both projections. Preserve the
existing source-key generation guard and prune superseded entries only after their display evidence has
been transferred or intentionally discarded.
### Relation and Changes projections
`apps/web/hooks/domains/github/use-pr-commits.ts` and
`apps/web/hooks/domains/session/use-remote-contribution-relation.ts` expose both projections.
`classifyRemoteContribution` receives only authoritative current evidence.
`useChangesPanelPRData` in `apps/web/components/task/changes-panel-data.tsx` receives retained display
commits. This explicit split prevents a stale commit list from enabling a remote action.
The existing `mergeCommits` and `CommitStatusMarker` behavior remains unchanged. Matching display SHAs
continue to show as pushed. If an accepted refresh removes a SHA, its presentation changes with that
snapshot.
### Desktop and mobile contract
Desktop and phone Changes surfaces already share `useChangesPanelData`, `mergeCommits`, and
`CommitRow`. This correction changes only their shared state projection. The nearest shipped mobile
surface is `apps/web/components/task/task-layout.tsx`. Existing Changes provenance coverage is in
`apps/web/e2e/tests/git/mobile-pr-checkout-drift.spec.ts`. There is no new entry point, hierarchy,
overlay, scroll owner, safe-area rule, touch target, or viewport-dependent interaction. Focused
hook/component coverage plus the existing shared-path browser scenario satisfies mobile parity. A
mobile-specific Playwright case is not required unless implementation changes responsive composition.
## Tests
- `apps/web/hooks/domains/github/use-pr-commits.test.ts`: add
`retains resolved display commits while a same-PR sync-version refresh is pending`. This is the
required fail-before-fix regression: current code returns an empty commit list after rerendering with
a new refresh key.
- In the same file, prove a failed same-identity refresh retains display commits, a successful refresh
replaces them, and a different pull request never receives them.
- `apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx`: prove that retained display
commits reach Changes while current evidence classifies as `unknown`. Prove that remote actions remain
disabled.
- `apps/web/components/task/changes-panel-remote.test.ts`: prove that retained display evidence keeps a
matching local commit pushed. It becomes unpushed only after an accepted current list omits it.
## E2E tests
- Extend `apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts` for
`AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7`. Seed a provider commit whose SHA matches checkout history.
Then switch to another task and create a new evidence version. Force both provider attempts to fail
before the switch back. Assert that the retained commit row still has pushed provenance. The hook
regression holds a request unresolved and covers the pending interval without an arbitrary timeout.
## Work orders
- [x] [Task 01: Stabilize commit provenance across refreshes](task-01-stabilize-commit-provenance.md)
## Verification results
- The provider commit resource retains the last successful display list for the selected contribution
identity. A separate authoritative list stays empty during a pending or failed refresh.
- The relation classifier uses only authoritative current evidence. Therefore, retained display data
cannot enable Push, Pull, or contribution replacement.
- Focused tests pass with 30 assertions across the resource, relation hook, and Changes projection.
- The production-build Chromium task-switch scenario passes with a forced failed refresh.
- TypeScript typecheck, frontend lint, specification lint, and `git diff --check` pass.
## Risks
- Retaining by source key instead of stable contribution identity can preserve the flicker or leak
another pull request's history.
- Passing retained commits into relation classification can incorrectly enable a remote mutation after
the provider head changes.
- Pruning the prior version before transferring its successful display snapshot can reintroduce the
empty interval.
- A final-state browser test alone can miss a short pending transition. The hook regression holds the
request unresolved, and the browser scenario covers retention after both refresh attempts fail.
@@ -0,0 +1,111 @@
---
id: "01-stabilize-commit-provenance"
title: "Stabilize commit provenance across refreshes"
status: completed
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001
acceptance_criteria:
- AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.4
- AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7
system_design:
- ../../specs/tasks/system-design/remote-contribution-tasks.md
---
# Task 01: Stabilize Commit Provenance Across Refreshes
## Summary
Retain the last successful provider commit list for display while a newer evidence version for the same
pull request is pending or failed. Keep relation classification and all remote actions bound to current,
complete evidence, then prove the task-switch behavior through focused unit and browser regressions.
## In scope
- Add an explicit display-commit projection to the provider commit resource and hook result.
- Carry retained display commits only across source versions with the same stable contribution identity.
- Keep provider head, completeness, errors, and authoritative commits scoped to the current version.
- Route authoritative evidence to relation classification and display evidence to Changes reconciliation.
- Cover pending, failure, success replacement, identity switch, action safety, and task-switch rendering.
## Out of scope
- Provider snapshot persistence.
- Local Git status or backend commit-history changes.
- Commit marker design, translations, or responsive layout changes.
- General Git subscription deduplication.
## Acceptance
- A same-PR version refresh never replaces previous confirmed pushed markers with unpushed arrows while
the current request is pending or failed. A successful response replaces the retained display
snapshot.
- A different workspace, repository, or pull request cannot receive retained commits, and late responses
cannot replace the selected identity or version.
- Retained display commits never establish alignment or divergence and never enable Push, Pull, or
contribution replacement without complete current evidence.
## Verification
```bash
(cd apps && pnpm --filter @kandev/web test -- --run hooks/domains/github/use-pr-commits.test.ts hooks/domains/session/use-remote-contribution-relation.test.tsx components/task/changes-panel-remote.test.ts)
(cd apps/web && pnpm run typecheck)
(cd apps && pnpm --filter @kandev/web lint)
(cd apps/web && pnpm e2e:run tests/pr/pr-switcher-changes.spec.ts)
(git diff --check)
```
## Files likely touched
- `apps/web/hooks/domains/github/pr-commits-resource.ts`
- `apps/web/hooks/domains/github/use-pr-commits.ts`
- `apps/web/hooks/domains/github/use-pr-commits.test.ts`
- `apps/web/hooks/domains/session/use-remote-contribution-relation.ts`
- `apps/web/hooks/domains/session/use-remote-contribution-relation.test.tsx`
- `apps/web/components/task/changes-panel-data.tsx`
- `apps/web/components/task/changes-panel-remote.test.ts`
- `apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts`
If the existing mock API cannot hold the provider response pending, these test-support files can also
change:
- `apps/web/e2e/helpers/api-client.ts`
- The corresponding backend mock GitHub handler and test
## Dependencies
None.
## Risks
- A combined authoritative/display state can accidentally make stale provider evidence actionable.
- Cache pruning and rapid task switches can drop retained evidence or expose it under the wrong identity.
- The existing E2E mock may need a narrow deterministic latency control to observe the pending interval.
## Parallelism
`sequential`
## Inputs
- `REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001`, especially
`AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.4` and `.7`.
- `docs/specs/tasks/system-design/remote-contribution-tasks.md` provider-history and failure sections.
- `docs/decisions/2026-08-13-provider-history-changes-enrichment.md`.
- Existing stale-while-revalidate behavior in
`apps/web/hooks/domains/github/use-active-task-pr-files.ts` and its refresh-retention tests.
## Results
- Added separate display and authoritative commit lists to the provider resource.
- Retained display commits now follow the latest selected source version for each contribution identity.
A late response cannot become the retained source for a newer version.
- Pending and failed refreshes preserve pushed markers. A successful refresh replaces the retained list.
- Relation classification uses only complete current evidence. All remote actions stay disabled when
that evidence is pending or failed.
- Added focused coverage for pending, failed, successful, identity-switch, and late-response cases.
- Extended the Chromium task-switch scenario with a failed provider refresh and a pushed-marker check.
- No public docs change is needed because this correction changes no command, setting, label, or user
workflow.
@@ -2,7 +2,7 @@
status: active
system: tasks
created: 2026-08-04
updated: 2026-08-19
updated: 2026-09-03
owners:
- product
---
@@ -28,3 +28,4 @@ user intent for destructive replacement, and evidence-based version comparison.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.4:** When the checkout and provider history diverge, the system shall classify versions by repository, branch, commit identity, and ancestry evidence; it shall preserve the local task version and expose distinct provider/local actions without treating message or patch similarity as equality.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.5:** When a user chooses to replace the provider branch, the system shall require explicit confirmation and an exact provider-head lease; if the provider head changed, it shall leave both versions unchanged and request a fresh review.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.6:** When a user chooses the provider version, the system shall require a clean working tree, create a local recovery branch at the current task head, and reset to the confirmed provider head while reporting the recovery branch.
- **AC-TASKS-REMOTE-CONTRIBUTION-TASKS-001.7:** When Kandev refreshes provider history for the same contribution, the Changes panel shall keep the previous confirmed commit provenance visible until refreshed evidence replaces it. A pending or failed refresh shall not show those commits as newly unpushed. Retained evidence shall not authorize a remote mutation.
@@ -4,7 +4,7 @@ system: tasks
requirements:
- REQ-TASKS-REMOTE-CONTRIBUTION-TASKS-001
created: 2026-08-04
updated: 2026-08-24
updated: 2026-09-03
owners:
- product
---
@@ -81,6 +81,13 @@ Kandev must preserve both versions and ask for user intent before one version re
- Provider commit history is optional enrichment for the Changes panel. Kandev shares identical
provider reads across Changes consumers and retries one failed read. If the retry fails, the panel
keeps the checkout history and does not show a provider-history warning.
- Provider commit state separates the stable contribution identity from the provider sync version.
During a newer read for the same workspace, repository, and pull request, Changes can retain the
last successful commit list for display provenance. This rule also applies after a failed read. The
current provider head, completeness, loading state, and error remain version-specific. Thus,
retained display commits cannot prove alignment, divergence, or permission for a remote mutation.
A successful current response replaces the retained display list. A different workspace,
repository, or pull request never inherits it.
- Kandev reconciles provider and checkout commits by SHA. Shared commits keep the normal commit
marker. Provider-only commits use the current-PR color and label. Checkout-only commits in a
confirmed divergence use a separate local-checkout color and label. The accessible label carries
@@ -238,6 +245,7 @@ recovery branch name after a successful reset. Neither action appears in the age
| The user selects **Use PR version** with local file changes | Kandev does not reset the checkout. It asks the user to commit or discard the file changes first. |
| The user selects **Use PR version** and the fetch does not match the confirmed provider head | Kandev does not create a recovery branch or reset the checkout. It refreshes the provider state. |
| Current provider commits cannot be loaded | Kandev retries once, keeps the checkout history without a warning, and derives remote actions only from sufficient evidence. |
| A same-contribution provider refresh is pending or fails after a successful read | Changes retains the last confirmed commit provenance for display, while current-evidence action gates remain closed until refresh succeeds. |
| Effective Git credentials cannot dry-run a push to the source branch | The task remains durable, but the session does not start and exposes an actionable credential/collaboration error. |
| Contribution binding is missing, malformed, or an unknown version | Runtime preparation and managed source-scope issuance fail closed. |
| Agent attempts a normal create-PR action | Kandev reuses the existing association and does not open a second remote change. |
@@ -256,6 +264,10 @@ The original contribution `head_sha` remains creation-time provenance. It is not
provider rewrite. Live provider commits and Git status remain observed state. Drift detection does not
reset, rebase, merge, or replace either version without a direct user action.
The frontend provider-history resource can retain the latest successful commit list in memory across
same-contribution evidence versions. This display cache is not persisted. It does not retain
provider-head authority. The resource discards the cache when it evicts the bounded entry.
A recovery branch created by **Use PR version** remains in the task repository across backend
restarts. Kandev does not persist confirmation dialogs, provider snapshots, or replacement leases.
@@ -370,6 +382,15 @@ WHEN the Changes panel renders the checkout
THEN it retries the provider read once and keeps the local checkout history without a warning
AND it does not claim the branch was rewritten or compare commits by message or patch similarity
### Keep confirmed provenance stable during a provider refresh
GIVEN the Changes panel confirmed that checkout commits belong to an associated pull request
AND Kandev starts a newer provider-history read for that same pull request
WHEN the new read is pending or fails
THEN those commits retain their confirmed published presentation instead of appearing newly unpushed
AND Push, Pull, and replacement actions remain gated by the unavailable current evidence
AND a successful response replaces the retained presentation with the refreshed commit history
### Distinguish commits when the provider is ahead
GIVEN a complete provider history contains local HEAD and one newer provider commit