fix(sync): Prevent reopening closed issues/stories during single-task sync

src/core/github-sync.test.ts

@@ -1,4 +1,5 @@
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import nock from "nock";
 import {
   GitHubSyncService,
   getGitHubToken,
@@ -532,18 +533,22 @@ describe("GitHubSyncService", () => {
         });
         const store = createStore([task]);
 
+        // Single-task sync uses getIssue (not listIssues) since task already has issueNumber
         // Issue is already CLOSED on GitHub (was closed on another machine)
-        // The cache will report state: "closed"
-        githubMock.listIssues("test-owner", "test-repo", [
+        githubMock.getIssue(
+          "test-owner",
+          "test-repo",
+          42,
           createIssueFixture({
             number: 42,
             title: task.name,
             state: "closed",
             body: `<!-- dex:task:id:test-task -->`,
           }),
-        ]);
+        );
 
         // Update should keep the issue closed (not reopen it)
+        // The key assertion: update is called but doesn't include state: "open"
         githubMock.updateIssue(
           "test-owner",
           "test-repo",
@@ -563,6 +568,115 @@ describe("GitHubSyncService", () => {
         // but the issue stays closed on GitHub (we don't reopen it)
         expect(getGitHubMetadata(result)?.state).toBe("open");
       });
+
+      it("does not reopen closed issue when syncing incomplete task without cache", async () => {
+        // Critical test for the fix: when syncTask is called directly (not through syncAll),
+        // there's no issue cache, so getIssueChangeResult must fetch the current state.
+        // If the remote issue is closed, we must not reopen it by sending state: "open".
+        const task = createTask({
+          id: "incomplete-task",
+          name: "Incomplete Task",
+          completed: false, // Task is NOT completed locally
+          metadata: {
+            github: {
+              issueNumber: 99,
+              issueUrl: "https://github.com/test-owner/test-repo/issues/99",
+              repo: "test-owner/test-repo",
+              state: "open", // Stale local metadata
+            },
+          },
+        });
+        const store = createStore([task]);
+
+        // The GitHub issue is already CLOSED (closed externally or by another machine)
+        githubMock.getIssue(
+          "test-owner",
+          "test-repo",
+          99,
+          createIssueFixture({
+            number: 99,
+            title: task.name,
+            state: "closed", // CLOSED on remote
+            body: `<!-- dex:task:id:incomplete-task -->`,
+          }),
+        );
+
+        // The update should NOT include state: "open" (would reopen the issue)
+        // Since the local content differs from remote, an update is needed
+        // but the state field should be omitted to preserve the closed state
+        githubMock.updateIssue(
+          "test-owner",
+          "test-repo",
+          99,
+          createIssueFixture({
+            number: 99,
+            title: task.name,
+            state: "closed", // Should stay closed
+          }),
+        );
+
+        const result = await service.syncTask(task, store);
+
+        expect(result).not.toBeNull();
+        // Expected state is "open" (task not completed), but issue should stay closed on GitHub
+        expect(getGitHubMetadata(result)?.state).toBe("open");
+      });
+
+      it("verifies request body does not contain state:open when issue is closed", async () => {
+        // This test uses nock body matching to PROVE we don't send state: "open"
+        const task = createTask({
+          id: "body-check-task",
+          name: "Body Check Task",
+          completed: false,
+          metadata: {
+            github: {
+              issueNumber: 88,
+              issueUrl: "https://github.com/test-owner/test-repo/issues/88",
+              repo: "test-owner/test-repo",
+              state: "open",
+            },
+          },
+        });
+        const store = createStore([task]);
+
+        // Setup getIssue to return closed issue
+        githubMock.getIssue(
+          "test-owner",
+          "test-repo",
+          88,
+          createIssueFixture({
+            number: 88,
+            title: task.name,
+            state: "closed",
+            body: `<!-- dex:task:id:body-check-task -->`,
+          }),
+        );
+
+        // Use nock directly with body matching to verify state is NOT "open"
+        let capturedBody: Record<string, unknown> | null = null;
+        nock("https://api.github.com")
+          .patch(`/repos/test-owner/test-repo/issues/88`, (body) => {
+            capturedBody = body as Record<string, unknown>;
+            // Accept any body - we'll verify after
+            return true;
+          })
+          .reply(200, {
+            number: 88,
+            title: task.name,
+            state: "closed",
+            html_url: "https://github.com/test-owner/test-repo/issues/88",
+          });
+
+        await service.syncTask(task, store);
+
+        // THE CRITICAL ASSERTION: state should NOT be "open"
+        expect(capturedBody).not.toBeNull();
+        expect(capturedBody!.state).not.toBe("open");
+        // state should either be undefined (not sent) or "closed"
+        expect(
+          capturedBody!.state === undefined || capturedBody!.state === "closed",
+        ).toBe(true);
+      });
     });
   });
 

src/core/github/sync.ts

@@ -306,8 +306,10 @@ export class GitHubSyncService {
       }
 
       // Get cached data for change detection and current state
+      // IMPORTANT: When state is unknown (no cache), use undefined to preserve remote state
+      // This prevents reopening closed issues when syncing a single task without cache
       const cached = issueCache?.get(parent.id);
-      const currentState = cached?.state ?? "open";
+      let currentState: "open" | "closed" | undefined = cached?.state;
 
       // Check if remote is newer than local (staleness detection)
       // If so, pull remote state to local instead of pushing
@@ -369,21 +371,27 @@ export class GitHubSyncService {
         const expectedBody = this.renderBody(parent, descendants);
         const expectedLabels = this.buildLabels(parent, shouldClose);
 
-        const hasChanges = cached
-          ? this.hasIssueChangedFromCache(
-              cached,
-              parent.name,
-              expectedBody,
-              expectedLabels,
-              shouldClose,
-            )
-          : await this.hasIssueChanged(
-              issueNumber,
-              parent.name,
-              expectedBody,
-              expectedLabels,
-              shouldClose,
-            );
+        let hasChanges: boolean;
+        if (cached) {
+          hasChanges = this.hasIssueChangedFromCache(
+            cached,
+            parent.name,
+            expectedBody,
+            expectedLabels,
+            shouldClose,
+          );
+        } else {
+          // No cache - need to fetch from API to get current state
+          const changeResult = await this.getIssueChangeResult(
+            issueNumber,
+            parent.name,
+            expectedBody,
+            expectedLabels,
+            shouldClose,
+          );
+          hasChanges = changeResult.hasChanges;
+          currentState = changeResult.currentState;
+        }
 
         if (!hasChanges) {
           onProgress?.({
@@ -400,6 +408,10 @@ export class GitHubSyncService {
             true,
           );
         }
+      } else if (!cached) {
+        // skipUnchanged is false and no cache - still need to fetch current state
+        // to avoid accidentally reopening closed issues
+        currentState = await this.fetchIssueState(issueNumber);
       }
 
       onProgress?.({
@@ -453,16 +465,20 @@ export class GitHubSyncService {
   }
 
   /**
-   * Check if an issue has changed compared to what we would push.
-   * Returns true if the issue needs updating.
+   * Check if an issue has changed and get its current state.
+   * Returns both change detection result and current state for safe updates.
+   * When we can't fetch the issue, currentState is undefined to preserve remote state.
    */
-  private async hasIssueChanged(
+  private async getIssueChangeResult(
     issueNumber: number,
     expectedTitle: string,
     expectedBody: string,
     expectedLabels: string[],
     shouldClose: boolean,
-  ): Promise<boolean> {
+  ): Promise<{
+    hasChanges: boolean;
+    currentState: "open" | "closed" | undefined;
+  }> {
     try {
       const { data: issue } = await this.octokit.issues.get({
         owner: this.owner,
@@ -474,7 +490,7 @@ export class GitHubSyncService {
         .map((l) => (typeof l === "string" ? l : l.name || ""))
         .filter((l) => l.startsWith(this.labelPrefix));
 
-      return this.issueNeedsUpdate(
+      const hasChanges = this.issueNeedsUpdate(
         {
           title: issue.title,
           body: issue.body || "",
@@ -486,9 +502,34 @@ export class GitHubSyncService {
         expectedLabels,
         shouldClose,
       );
+
+      return {
+        hasChanges,
+        currentState: issue.state as "open" | "closed",
+      };
     } catch {
       // If we can't fetch the issue, assume it needs updating
-      return true;
+      // but use undefined state to preserve whatever the remote state is
+      return { hasChanges: true, currentState: undefined };
+    }
+  }
+
+  /**
+   * Fetch only the state of an issue (for when we need to avoid reopening).
+   * Returns undefined if the issue can't be fetched.
+   */
+  private async fetchIssueState(
+    issueNumber: number,
+  ): Promise<"open" | "closed" | undefined> {
+    try {
+      const { data: issue } = await this.octokit.issues.get({
+        owner: this.owner,
+        repo: this.repo,
+        issue_number: issueNumber,
+      });
+      return issue.state as "open" | "closed";
+    } catch {
+      return undefined;
     }
   }
 
@@ -570,30 +611,32 @@ export class GitHubSyncService {
    * Update an existing GitHub issue.
    *
    * @param currentState - The current state of the issue on GitHub.
+   *                       undefined means we don't know the current state.
    *                       Used to prevent reopening closed issues.
    */
   private async updateIssue(
     parent: Task,
     descendants: HierarchicalTask[],
     issueNumber: number,
     shouldClose: boolean,
-    currentState: "open" | "closed",
+    currentState: "open" | "closed" | undefined,
   ): Promise<void> {
     const body = this.renderBody(parent, descendants);
 
     // Determine the state to set:
     // - If shouldClose is true, always close (even if already closed)
     // - If shouldClose is false and currently open, keep open
     // - If shouldClose is false and currently closed, DON'T reopen
-    //   (this prevents reopening issues that were closed elsewhere)
+    // - If shouldClose is false and state is unknown (undefined), don't set state
+    //   (this preserves whatever the remote state is, preventing accidental reopening)
     let state: "open" | "closed" | undefined;
     if (shouldClose) {
       state = "closed";
     } else if (currentState === "open") {
       state = "open";
     }
-    // If currentState is "closed" and shouldClose is false, don't set state
-    // (keeps the issue closed, doesn't reopen it)
+    // If currentState is "closed" or undefined and shouldClose is false, don't set state
+    // (keeps the issue in its current state, doesn't reopen it)
 
     await this.octokit.issues.update({
       owner: this.owner,