fix(github): dedupe task progress updates
Post at most one in-progress comment per Fusion task while keeping failed deliveries retryable. Persist a durable marker and fall back to the task log when local marker storage fails after GitHub accepts the comment.
This commit is contained in:
7
.changeset/single-github-progress-update.md
Normal file
7
.changeset/single-github-progress-update.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Send only one in-progress update per Fusion task on its linked GitHub tracking issue.
|
||||||
|
category: fix
|
||||||
|
dev: Persists the successful in-progress notification marker and retains legacy task-log deduplication.
|
||||||
@@ -113,6 +113,8 @@ export interface TaskGithubTracking {
|
|||||||
repoOverride?: string;
|
repoOverride?: string;
|
||||||
/** Linked GitHub issue. Set after issue creation succeeds. Cleared via unlinkGithubIssue(). */
|
/** Linked GitHub issue. Set after issue creation succeeds. Cleared via unlinkGithubIssue(). */
|
||||||
issue?: TaskGithubTrackedIssue;
|
issue?: TaskGithubTrackedIssue;
|
||||||
|
/** ISO-8601 timestamp of the task's one permitted in-progress tracking comment. */
|
||||||
|
inProgressCommentedAt?: string;
|
||||||
/** ISO-8601 of the most recent manual unlink, retained for audit. */
|
/** ISO-8601 of the most recent manual unlink, retained for audit. */
|
||||||
unlinkedAt?: string;
|
unlinkedAt?: string;
|
||||||
}
|
}
|
||||||
@@ -142,4 +144,3 @@ export interface TaskSourceIssue {
|
|||||||
*/
|
*/
|
||||||
closedAt?: string;
|
closedAt?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ vi.mock("../cli-package-version.js", async (importOriginal) => ({
|
|||||||
class MockStore extends EventEmitter {
|
class MockStore extends EventEmitter {
|
||||||
logEntry: Mock;
|
logEntry: Mock;
|
||||||
getTask: Mock;
|
getTask: Mock;
|
||||||
|
updateTask: Mock;
|
||||||
getSettings: Mock;
|
getSettings: Mock;
|
||||||
getGlobalSettingsStore: Mock;
|
getGlobalSettingsStore: Mock;
|
||||||
|
|
||||||
@@ -50,6 +51,7 @@ class MockStore extends EventEmitter {
|
|||||||
this.logEntry = vi.fn().mockResolvedValue(undefined);
|
this.logEntry = vi.fn().mockResolvedValue(undefined);
|
||||||
// Null preserves the event snapshot unless a test supplies a newer authoritative row.
|
// Null preserves the event snapshot unless a test supplies a newer authoritative row.
|
||||||
this.getTask = vi.fn().mockResolvedValue(null);
|
this.getTask = vi.fn().mockResolvedValue(null);
|
||||||
|
this.updateTask = vi.fn().mockResolvedValue(undefined);
|
||||||
this.getSettings = vi.fn().mockResolvedValue({ githubAuthMode: "token", githubAuthToken: "ghp_test" });
|
this.getSettings = vi.fn().mockResolvedValue({ githubAuthMode: "token", githubAuthToken: "ghp_test" });
|
||||||
this.getGlobalSettingsStore = vi.fn(() => ({ getSettings: vi.fn().mockResolvedValue({}) }));
|
this.getGlobalSettingsStore = vi.fn(() => ({ getSettings: vi.fn().mockResolvedValue({}) }));
|
||||||
}
|
}
|
||||||
@@ -572,18 +574,121 @@ describe("GitHubTrackingCommentService", () => {
|
|||||||
expect(mockCommentOnIssue.mock.calls[0]?.[3]).not.toContain("Commit:");
|
expect(mockCommentOnIssue.mock.calls[0]?.[3]).not.toContain("Commit:");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not refetch or duplicate a comment for in-progress and same-column transitions", async () => {
|
it("does not duplicate a comment for in-progress and same-column transitions", async () => {
|
||||||
service.start();
|
service.start();
|
||||||
|
|
||||||
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
store.emit("task:moved", { task: createTask(), from: "done", to: "done" });
|
store.emit("task:moved", { task: createTask(), from: "done", to: "done" });
|
||||||
await flushAsync();
|
await flushAsync();
|
||||||
|
|
||||||
expect(store.getTask).not.toHaveBeenCalled();
|
expect(store.getTask).toHaveBeenCalledTimes(1);
|
||||||
expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
|
expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
|
||||||
expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain("🚧 In progress");
|
expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain("🚧 In progress");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("posts only one in-progress comment when a task leaves and re-enters the column", async () => {
|
||||||
|
service.start();
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
|
||||||
|
expect(mockCommentOnIssue.mock.calls[0]?.[3]).toContain("🚧 In progress");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not repost an in-progress comment after the service restarts", async () => {
|
||||||
|
service.start();
|
||||||
|
service.stop();
|
||||||
|
service = new GitHubTrackingCommentService(store as unknown as TaskStore);
|
||||||
|
service.start();
|
||||||
|
|
||||||
|
store.getTask.mockResolvedValueOnce(createTask({
|
||||||
|
githubTracking: {
|
||||||
|
enabled: true,
|
||||||
|
inProgressCommentedAt: "2026-07-21T12:00:00.000Z",
|
||||||
|
issue: {
|
||||||
|
owner: "owner",
|
||||||
|
repo: "repo",
|
||||||
|
number: 42,
|
||||||
|
url: "https://github.com/owner/repo/issues/42",
|
||||||
|
createdAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not repost for legacy tasks whose success is recorded only in the task log", async () => {
|
||||||
|
service.start();
|
||||||
|
store.getTask.mockResolvedValueOnce(createTask({
|
||||||
|
log: [{
|
||||||
|
timestamp: "2026-07-21T12:00:00.000Z",
|
||||||
|
action: "Posted GitHub tracking comment",
|
||||||
|
outcome: "owner/repo#42 (in-progress)",
|
||||||
|
}],
|
||||||
|
}));
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows a later in-progress transition to retry after GitHub rejects the first post", async () => {
|
||||||
|
service.start();
|
||||||
|
mockCommentOnIssue.mockRejectedValueOnce(new Error("rate limited"));
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).toHaveBeenCalledTimes(2);
|
||||||
|
expect(store.updateTask).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("records durable success when the marker write fails after GitHub accepts the comment", async () => {
|
||||||
|
service.start();
|
||||||
|
store.updateTask.mockRejectedValueOnce(new Error("store unavailable"));
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
|
||||||
|
expect(store.logEntry).toHaveBeenCalledWith(
|
||||||
|
"FN-1",
|
||||||
|
"Posted GitHub tracking comment",
|
||||||
|
"owner/repo#42 (in-progress)",
|
||||||
|
);
|
||||||
|
expect(store.logEntry).not.toHaveBeenCalledWith(
|
||||||
|
"FN-1",
|
||||||
|
"Failed to post GitHub tracking comment",
|
||||||
|
"store unavailable",
|
||||||
|
);
|
||||||
|
|
||||||
|
service.stop();
|
||||||
|
service = new GitHubTrackingCommentService(store as unknown as TaskStore);
|
||||||
|
service.start();
|
||||||
|
store.getTask.mockResolvedValueOnce(createTask({
|
||||||
|
log: [{
|
||||||
|
timestamp: "2026-07-21T12:00:00.000Z",
|
||||||
|
action: "Posted GitHub tracking comment",
|
||||||
|
outcome: "owner/repo#42 (in-progress)",
|
||||||
|
}],
|
||||||
|
}));
|
||||||
|
|
||||||
|
store.emit("task:moved", { task: createTask(), from: "todo", to: "in-progress" });
|
||||||
|
await flushAsync();
|
||||||
|
|
||||||
|
expect(mockCommentOnIssue).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
it("writes success logs", async () => {
|
it("writes success logs", async () => {
|
||||||
service.start();
|
service.start();
|
||||||
|
|
||||||
|
|||||||
@@ -186,6 +186,7 @@ export function formatTrackingComment(
|
|||||||
|
|
||||||
export class GitHubTrackingCommentService {
|
export class GitHubTrackingCommentService {
|
||||||
private readonly store: TaskStore;
|
private readonly store: TaskStore;
|
||||||
|
private readonly inProgressCommentClaims = new Set<string>();
|
||||||
private readonly onTaskMoved = (event: TaskMovedEvent): void => {
|
private readonly onTaskMoved = (event: TaskMovedEvent): void => {
|
||||||
void this.handleTaskMoved(event);
|
void this.handleTaskMoved(event);
|
||||||
};
|
};
|
||||||
@@ -248,25 +249,46 @@ export class GitHubTrackingCommentService {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (event.to === "in-progress") {
|
||||||
|
if (this.inProgressCommentClaims.has(event.task.id)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
this.inProgressCommentClaims.add(event.task.id);
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* FNXC:GitHubTrackingComments 2026-07-16-12:40:
|
* FNXC:GitHubTrackingComments 2026-07-16-12:40:
|
||||||
* A closed tracked issue must link its landing commit when one exists. The task:moved snapshot
|
* A closed tracked issue must link its landing commit when one exists, and in-progress comments
|
||||||
* can predate mergeDetails persistence on human PR, no-op, and recovery done paths, so re-read
|
* must honor the durable one-per-task marker. Re-read the authoritative row before either
|
||||||
* the authoritative row before building the Done comment. Fall back to the snapshot when the
|
* transition; fall back to the event snapshot when the read fails so the comment is not dropped.
|
||||||
* read fails so the comment is never dropped.
|
|
||||||
*/
|
*/
|
||||||
const taskForComment = event.to === "done"
|
const authoritativeTask = await this.store.getTask(event.task.id).catch(() => null);
|
||||||
? await this.store.getTask(event.task.id).catch(() => null) ?? event.task
|
const taskForComment = authoritativeTask ?? event.task;
|
||||||
: event.task;
|
if (
|
||||||
|
event.to === "in-progress"
|
||||||
|
&& (
|
||||||
|
taskForComment.githubTracking?.inProgressCommentedAt
|
||||||
|
|| taskForComment.log?.some((entry) => (
|
||||||
|
entry.action === "Posted GitHub tracking comment"
|
||||||
|
&& entry.outcome?.endsWith("(in-progress)")
|
||||||
|
))
|
||||||
|
)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
const body = event.to === "done"
|
const body = event.to === "done"
|
||||||
? formatTrackingComment(taskForComment, event.to, { owner, repo })
|
? formatTrackingComment(taskForComment, event.to, { owner, repo })
|
||||||
: formatTrackingComment(taskForComment, event.to);
|
: formatTrackingComment(taskForComment, event.to);
|
||||||
|
|
||||||
|
let commentPosted = false;
|
||||||
try {
|
try {
|
||||||
const projectSettings = await this.store.getSettings() as Pick<ProjectSettings, "githubAuthMode" | "githubAuthToken">;
|
const projectSettings = await this.store.getSettings() as Pick<ProjectSettings, "githubAuthMode" | "githubAuthToken">;
|
||||||
const globalSettings = (await this.store.getGlobalSettingsStore?.()?.getSettings?.() ?? {}) as Pick<GlobalSettings, never>;
|
const globalSettings = (await this.store.getGlobalSettingsStore?.()?.getSettings?.() ?? {}) as Pick<GlobalSettings, never>;
|
||||||
const resolution = resolveGithubTrackingAuth({ projectSettings, globalSettings });
|
const resolution = resolveGithubTrackingAuth({ projectSettings, globalSettings });
|
||||||
if (!resolution.ok) {
|
if (!resolution.ok) {
|
||||||
|
if (event.to === "in-progress") {
|
||||||
|
this.inProgressCommentClaims.delete(event.task.id);
|
||||||
|
}
|
||||||
await this.safeLogDeletedTaskEntry(event.task.id, "Skipped GitHub tracking comment", resolution.message);
|
await this.safeLogDeletedTaskEntry(event.task.id, "Skipped GitHub tracking comment", resolution.message);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -275,12 +297,33 @@ export class GitHubTrackingCommentService {
|
|||||||
? 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.commentOnIssue(owner, repo, number, body);
|
await client.commentOnIssue(owner, repo, number, body);
|
||||||
|
commentPosted = true;
|
||||||
|
if (event.to === "in-progress") {
|
||||||
|
try {
|
||||||
|
await this.store.updateTask(event.task.id, {
|
||||||
|
githubTracking: { inProgressCommentedAt: new Date().toISOString() },
|
||||||
|
});
|
||||||
|
} catch (markerError) {
|
||||||
|
await this.safeLogDeletedTaskEntry(
|
||||||
|
event.task.id,
|
||||||
|
"Posted GitHub tracking comment",
|
||||||
|
`${owner}/${repo}#${number} (${event.to})`,
|
||||||
|
);
|
||||||
|
console.warn(
|
||||||
|
`[github-tracking-comments] Posted in-progress comment for ${event.task.id}, but failed to persist its marker: ${markerError instanceof Error ? markerError.message : String(markerError)}`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
await this.safeLogDeletedTaskEntry(
|
await this.safeLogDeletedTaskEntry(
|
||||||
event.task.id,
|
event.task.id,
|
||||||
"Posted GitHub tracking comment",
|
"Posted GitHub tracking comment",
|
||||||
`${owner}/${repo}#${number} (${event.to})`,
|
`${owner}/${repo}#${number} (${event.to})`,
|
||||||
);
|
);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
if (event.to === "in-progress" && !commentPosted) {
|
||||||
|
this.inProgressCommentClaims.delete(event.task.id);
|
||||||
|
}
|
||||||
const message = err instanceof Error ? err.message : String(err);
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
await this.safeLogDeletedTaskEntry(
|
await this.safeLogDeletedTaskEntry(
|
||||||
event.task.id,
|
event.task.id,
|
||||||
|
|||||||
Reference in New Issue
Block a user