fix: make sure cancel actually aborts push

[jonathanlab]

Now that we immediately push on push & commit we should let the user abort the push with the cancel button in the UI. HOWEVER we do not support canceling TRPC procedures via electron-trpc so we had to add that in as well.

[jonathanlab]

• feat: ability to mention folders #1841
• feat: convert pasted links to gh chip, support PRs #1838
• feat: mentions gh issues/pr with # #1834
• fix: let chat pane resize to min width #1826
• fix: make sure cancel actually aborts push #1831 👈 (View in Graphite)
• fix: immediately run push on commit & push #1830
• fix: increase sidebar min width #1824
• fix(git): persist pr draft values #1823
• fix: handle multiline chips, unescape xml #1818
• fix: unify send message submit key behavior #1817
• fix: add back shift-tab support #1805
• fix: handle git-lfs with GIT_LFS_SKIP_SMUDGE #1804
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Comments Outside Diff (1)

1. packages/electron-trpc/src/main/handleIPCMessage.ts, line 211-226 (link)

   operations.delete incorrectly removes a legitimate operation on duplicate-ID error

   When the duplicate-ID check throws (line 65-70), operations.set has not been called for the current invocation — the existing first operation's controller is registered under internalId. The unconditional operations.delete(internalId) in the catch block then evicts the first operation's AbortController, leaving it permanently un-cancellable for the rest of its lifetime.

   Track whether set was called in this invocation and guard the delete accordingly:

     let registered = false;
     try {
       if (operations.has(internalId)) {
         throw new TRPCError({ message: `Duplicate id ${internalId}`, code: "BAD_REQUEST" });
       }
       operations.set(internalId, abortController);
       registered = true;
       // ...
     } catch (cause) {
       if (registered) {
         operations.delete(internalId);
       }
       // ...
     }
   

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: packages/electron-trpc/src/main/handleIPCMessage.ts
   Line: 211-226
   
   Comment:
   **`operations.delete` incorrectly removes a legitimate operation on duplicate-ID error**
   
   When the duplicate-ID check throws (line 65-70), `operations.set` has **not** been called for the current invocation — the existing first operation's controller is registered under `internalId`. The unconditional `operations.delete(internalId)` in the catch block then evicts the first operation's `AbortController`, leaving it permanently un-cancellable for the rest of its lifetime.
   
   Track whether `set` was called in this invocation and guard the delete accordingly:
   
   ```
     let registered = false;
     try {
       if (operations.has(internalId)) {
         throw new TRPCError({ message: `Duplicate id ${internalId}`, code: "BAD_REQUEST" });
       }
       operations.set(internalId, abortController);
       registered = true;
       // ...
     } catch (cause) {
       if (registered) {
         operations.delete(internalId);
       }
       // ...
     }
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: packages/electron-trpc/src/main/handleIPCMessage.ts
Line: 211-226

Comment:
**`operations.delete` incorrectly removes a legitimate operation on duplicate-ID error**

When the duplicate-ID check throws (line 65-70), `operations.set` has **not** been called for the current invocation — the existing first operation's controller is registered under `internalId`. The unconditional `operations.delete(internalId)` in the catch block then evicts the first operation's `AbortController`, leaving it permanently un-cancellable for the rest of its lifetime.

Track whether `set` was called in this invocation and guard the delete accordingly:

```
  let registered = false;
  try {
    if (operations.has(internalId)) {
      throw new TRPCError({ message: `Duplicate id ${internalId}`, code: "BAD_REQUEST" });
    }
    operations.set(internalId, abortController);
    registered = true;
    // ...
  } catch (cause) {
    if (registered) {
      operations.delete(internalId);
    }
    // ...
  }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/electron-trpc/src/renderer/ipcLink.ts
Line: 96-102

Comment:
**Aborted operation may wait forever if cancel races context creation**

`operations.set` only happens after `await createContext()` returns (line 52 → 71 in `handleIPCMessage.ts`). If the `AbortSignal` fires and `operation.cancel` is dispatched during that `await`, `operations.get(internalId)` returns `undefined` and the abort is silently dropped. The renderer then waits indefinitely for a response that will never arrive, because the procedure itself runs to completion uninterrupted.

A small hardening in `handleIPCMessage.ts` would close it: move `operations.set(internalId, abortController)` to immediately before `await createContext()`, or check `abortController.signal.aborted` right after `createContext` returns.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/code/src/renderer/features/git-interaction/hooks/useGitInteraction.ts
Line: 463-476

Comment:
**`trackGitAction` duplicated across both catch branches**

`trackGitAction(taskId, store.pushMode, false)` appears identically in both the aborted path and the error path. Per the OnceAndOnlyOnce principle, it can be hoisted before the branch:

```suggestion
    } catch (error) {
      trackGitAction(taskId, store.pushMode, false);
      if (controller.signal.aborted) {
        return;
      }
      log.error("Push failed", error);
      const message = error instanceof Error ? error.message : "Push failed.";
      modal.setPushError(message);
      modal.setPushState("error");
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: make sure cancel actually aborts pu..." | Re-trigger Greptile}

[greptile-apps]

Aborted operation may wait forever if cancel races context creation

operations.set only happens after await createContext() returns (line 52 → 71 in handleIPCMessage.ts). If the AbortSignal fires and operation.cancel is dispatched during that await, operations.get(internalId) returns undefined and the abort is silently dropped. The renderer then waits indefinitely for a response that will never arrive, because the procedure itself runs to completion uninterrupted.

A small hardening in handleIPCMessage.ts would close it: move operations.set(internalId, abortController) to immediately before await createContext(), or check abortController.signal.aborted right after createContext returns.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/electron-trpc/src/renderer/ipcLink.ts
Line: 96-102

Comment:
**Aborted operation may wait forever if cancel races context creation**

`operations.set` only happens after `await createContext()` returns (line 52 → 71 in `handleIPCMessage.ts`). If the `AbortSignal` fires and `operation.cancel` is dispatched during that `await`, `operations.get(internalId)` returns `undefined` and the abort is silently dropped. The renderer then waits indefinitely for a response that will never arrive, because the procedure itself runs to completion uninterrupted.

A small hardening in `handleIPCMessage.ts` would close it: move `operations.set(internalId, abortController)` to immediately before `await createContext()`, or check `abortController.signal.aborted` right after `createContext` returns.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

trackGitAction duplicated across both catch branches

trackGitAction(taskId, store.pushMode, false) appears identically in both the aborted path and the error path. Per the OnceAndOnlyOnce principle, it can be hoisted before the branch:

Suggested change

	modal.setPushState("success");
	} catch (error) {
	if (controller.signal.aborted) {
	trackGitAction(taskId, store.pushMode, false);
	return;
	}
	log.error("Push failed", error);
	const message = error instanceof Error ? error.message : "Push failed.";
	trackGitAction(taskId, store.pushMode, false);
	modal.setPushError(message);
	modal.setPushState("error");
	} finally {
	if (pushAbortRef.current === controller) {
	} catch (error) {
	trackGitAction(taskId, store.pushMode, false);
	if (controller.signal.aborted) {
	return;
	}
	log.error("Push failed", error);
	const message = error instanceof Error ? error.message : "Push failed.";
	modal.setPushError(message);
	modal.setPushState("error");

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/code/src/renderer/features/git-interaction/hooks/useGitInteraction.ts
Line: 463-476

Comment:
**`trackGitAction` duplicated across both catch branches**

`trackGitAction(taskId, store.pushMode, false)` appears identically in both the aborted path and the error path. Per the OnceAndOnlyOnce principle, it can be hoisted before the branch:

```suggestion
    } catch (error) {
      trackGitAction(taskId, store.pushMode, false);
      if (controller.signal.aborted) {
        return;
      }
      log.error("Push failed", error);
      const message = error instanceof Error ? error.message : "Push failed.";
      modal.setPushError(message);
      modal.setPushState("error");
```

How can I resolve this? If you propose a fix, please make it concise.

[charlesvien]

Merge activity

• Apr 23, 11:10 AM UTC: @charlesvien merged this pull request with Graphite.