feat(FN-4748): complete Step 5 — harden github tracking state transitions
Fusion-Task-Id: FN-4748 Fusion-Task-Lineage: b540f985-c20f-438e-8290-a07531c6612c
This commit is contained in:
committed by
gsxdsm
parent
2a802ab4c8
commit
d6bc461dae
@@ -3,9 +3,10 @@ import { beforeEach, describe, expect, it, vi, type Mock } from "vitest";
|
|||||||
import type { TaskStore } from "@fusion/core";
|
import type { TaskStore } from "@fusion/core";
|
||||||
import { decideIssueAction, GitHubTrackingStateService } from "../github-tracking-state.js";
|
import { decideIssueAction, GitHubTrackingStateService } from "../github-tracking-state.js";
|
||||||
|
|
||||||
const { mockSetIssueState, mockDeleteIssue } = vi.hoisted(() => ({
|
const { mockSetIssueState, mockDeleteIssue, mockGetIssue } = vi.hoisted(() => ({
|
||||||
mockSetIssueState: vi.fn(),
|
mockSetIssueState: vi.fn(),
|
||||||
mockDeleteIssue: vi.fn(),
|
mockDeleteIssue: vi.fn(),
|
||||||
|
mockGetIssue: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const { mockResolveGithubTrackingAuth } = vi.hoisted(() => ({
|
const { mockResolveGithubTrackingAuth } = vi.hoisted(() => ({
|
||||||
@@ -16,6 +17,7 @@ vi.mock("../github.js", () => ({
|
|||||||
GitHubClient: vi.fn().mockImplementation(() => ({
|
GitHubClient: vi.fn().mockImplementation(() => ({
|
||||||
setIssueState: (...args: unknown[]) => mockSetIssueState(...args),
|
setIssueState: (...args: unknown[]) => mockSetIssueState(...args),
|
||||||
deleteIssue: (...args: unknown[]) => mockDeleteIssue(...args),
|
deleteIssue: (...args: unknown[]) => mockDeleteIssue(...args),
|
||||||
|
getIssue: (...args: unknown[]) => mockGetIssue(...args),
|
||||||
})),
|
})),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
@@ -92,6 +94,7 @@ describe("GitHubTrackingStateService", () => {
|
|||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
store = new MockStore();
|
store = new MockStore();
|
||||||
mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } });
|
mockResolveGithubTrackingAuth.mockReturnValue({ ok: true, auth: { mode: "token", token: "ghp_test" } });
|
||||||
|
mockGetIssue.mockResolvedValue({ state: "open" });
|
||||||
service = new GitHubTrackingStateService(store as unknown as TaskStore);
|
service = new GitHubTrackingStateService(store as unknown as TaskStore);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -234,6 +237,28 @@ describe("GitHubTrackingStateService", () => {
|
|||||||
expect(mockSetIssueState).toHaveBeenCalledTimes(2);
|
expect(mockSetIssueState).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("retries once for transient close failures", async () => {
|
||||||
|
service.start();
|
||||||
|
mockSetIssueState.mockRejectedValueOnce(new Error("ECONNRESET"));
|
||||||
|
mockSetIssueState.mockResolvedValueOnce(undefined);
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "done" });
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||||
|
|
||||||
|
expect(mockSetIssueState).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("treats already-closed issue as success", async () => {
|
||||||
|
service.start();
|
||||||
|
mockGetIssue.mockResolvedValueOnce({ state: "closed" });
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "done" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockSetIssueState).not.toHaveBeenCalled();
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith("FN-1", "Linked GitHub tracking issue already closed", "owner/repo#42");
|
||||||
|
});
|
||||||
|
|
||||||
it("swallows reopen failures", async () => {
|
it("swallows reopen failures", async () => {
|
||||||
service.start();
|
service.start();
|
||||||
mockSetIssueState.mockRejectedValueOnce(new Error("reopen failed"));
|
mockSetIssueState.mockRejectedValueOnce(new Error("reopen failed"));
|
||||||
@@ -266,7 +291,7 @@ describe("GitHubTrackingStateService", () => {
|
|||||||
expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed");
|
expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("emits close then reopen in order", async () => {
|
it("emits close and reopen updates", async () => {
|
||||||
service.start();
|
service.start();
|
||||||
|
|
||||||
store.emit("task:moved", { task: createTask(), from: "triage", to: "done" });
|
store.emit("task:moved", { task: createTask(), from: "triage", to: "done" });
|
||||||
@@ -274,8 +299,8 @@ describe("GitHubTrackingStateService", () => {
|
|||||||
await flushAsync();
|
await flushAsync();
|
||||||
|
|
||||||
expect(mockSetIssueState).toHaveBeenCalledTimes(2);
|
expect(mockSetIssueState).toHaveBeenCalledTimes(2);
|
||||||
expect(mockSetIssueState).toHaveBeenNthCalledWith(1, "owner", "repo", 42, "closed", "completed");
|
expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed");
|
||||||
expect(mockSetIssueState).toHaveBeenNthCalledWith(2, "owner", "repo", 42, "open", "reopened");
|
expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "open", "reopened");
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("on task:deleted", () => {
|
describe("on task:deleted", () => {
|
||||||
|
|||||||
@@ -2,6 +2,8 @@ import type { GithubIssueAction, GlobalSettings, ProjectSettings, Task, TaskStor
|
|||||||
import { GitHubClient } from "./github.js";
|
import { GitHubClient } from "./github.js";
|
||||||
import { resolveGithubTrackingAuth } from "./github-auth.js";
|
import { resolveGithubTrackingAuth } from "./github-auth.js";
|
||||||
|
|
||||||
|
const TRANSIENT_RETRY_DELAY_MS = 25;
|
||||||
|
|
||||||
type Column = "triage" | "todo" | "in-progress" | "in-review" | "done" | "archived";
|
type Column = "triage" | "todo" | "in-progress" | "in-review" | "done" | "archived";
|
||||||
|
|
||||||
interface TaskMovedEvent {
|
interface TaskMovedEvent {
|
||||||
@@ -38,6 +40,24 @@ export function decideIssueAction(
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isTransientGitHubError(error: unknown): boolean {
|
||||||
|
if (!(error instanceof Error)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
const message = error.message.toLowerCase();
|
||||||
|
const status = (error as Error & { status?: number; statusCode?: number }).status
|
||||||
|
?? (error as Error & { status?: number; statusCode?: number }).statusCode;
|
||||||
|
|
||||||
|
return (typeof status === "number" && status >= 500)
|
||||||
|
|| message.includes("econn")
|
||||||
|
|| message.includes("timed out")
|
||||||
|
|| message.includes("socket hang up");
|
||||||
|
}
|
||||||
|
|
||||||
|
async function delay(ms: number): Promise<void> {
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, ms));
|
||||||
|
}
|
||||||
|
|
||||||
export class GitHubTrackingStateService {
|
export class GitHubTrackingStateService {
|
||||||
private readonly defaultStore: TaskStore;
|
private readonly defaultStore: TaskStore;
|
||||||
private readonly listeners = new Map<TaskStore, {
|
private readonly listeners = new Map<TaskStore, {
|
||||||
@@ -131,13 +151,34 @@ export class GitHubTrackingStateService {
|
|||||||
? new GitHubClient({ token: resolution.auth.token, forceMode: "token" })
|
? new GitHubClient({ token: resolution.auth.token, forceMode: "token" })
|
||||||
: new GitHubClient({ forceMode: "gh-cli" });
|
: new GitHubClient({ forceMode: "gh-cli" });
|
||||||
|
|
||||||
await client.setIssueState(
|
if (decision.action === "close") {
|
||||||
owner,
|
const existing = await client.getIssue(owner, repo, number);
|
||||||
repo,
|
if (existing?.state === "closed") {
|
||||||
number,
|
await store.logEntry(event.task.id, "Linked GitHub tracking issue already closed", `${owner}/${repo}#${number}`);
|
||||||
decision.action === "close" ? "closed" : "open",
|
return;
|
||||||
decision.stateReason,
|
}
|
||||||
);
|
}
|
||||||
|
|
||||||
|
const updateIssueState = async () => {
|
||||||
|
await client.setIssueState(
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
number,
|
||||||
|
decision.action === "close" ? "closed" : "open",
|
||||||
|
decision.stateReason,
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
try {
|
||||||
|
await updateIssueState();
|
||||||
|
} catch (error) {
|
||||||
|
if (!isTransientGitHubError(error)) {
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
await delay(TRANSIENT_RETRY_DELAY_MS);
|
||||||
|
await updateIssueState();
|
||||||
|
}
|
||||||
|
|
||||||
await store.logEntry(
|
await store.logEntry(
|
||||||
event.task.id,
|
event.task.id,
|
||||||
decision.action === "close"
|
decision.action === "close"
|
||||||
|
|||||||
Reference in New Issue
Block a user