Skip to content

fix: make update check non-blocking by spawning detached child process#246

Draft
bukinoshita wants to merge 2 commits intomainfrom
fix/update-check-non-blocking-e9aa
Draft

fix: make update check non-blocking by spawning detached child process#246
bukinoshita wants to merge 2 commits intomainfrom
fix/update-check-non-blocking-e9aa

Conversation

@bukinoshita
Copy link
Copy Markdown
Member

@bukinoshita bukinoshita commented Apr 9, 2026

Summary

Fixes a bug where the CLI update notifier blocks command exit for up to ~5 seconds on slow/offline networks (BU-642).

After every successful non-update command, checkForUpdates() was awaiting a live GitHub fetch() with a 5-second AbortSignal.timeout whenever the cached update state was stale or missing. This kept the Node event loop alive and made fast commands appear to hang.

Changes

src/lib/update-check.ts

  • checkForUpdates() is now synchronous — it reads the cached state file, optionally prints a stale update notice, and when the cache is stale/missing, spawns a detached child process to refresh the cache in the background
  • spawnBackgroundRefresh() spawns a detached node -e child process with stdio: 'ignore' and .unref() so the main CLI process exits immediately
  • buildRefreshScript() generates a self-contained Node script that fetches the latest GitHub release and writes update-state.json, falling back to writing the current version on any failure (prevents repeated fetch retries on every subsequent command)
  • resolveNodePath() determines the correct Node binary path for spawning the child process
  • All functions converted to const arrow functions per coding standards
  • Removed JSDoc comments per coding standards

src/cli.ts

  • Removed .catch() wrapper since checkForUpdates() is no longer async and never throws

tests/lib/update-check.test.ts

  • Rewrote tests using vi.mock('node:child_process') to mock the spawn call in ESM
  • Added tests for the new background refresh behavior: spawn on stale cache, spawn on missing cache, stale notice display, skip conditions
  • Added dedicated test suites for buildRefreshScript, resolveNodePath, and spawnBackgroundRefresh
  • Converted from test() to it() per coding standards

How it works

Before: cli.ts → checkForUpdates() → await fetchLatestVersion() → fetch(..., timeout: 5s) — blocks process exit

After: cli.ts → checkForUpdates() → spawnBackgroundRefresh() — spawns a detached child process that fetches and writes the state file independently; main process exits immediately

The cached notice path (fresh cache) remains in-process and is unaffected. On the next command after a background refresh, the updated cache will be read and any update notice displayed.

Linear Issue: BU-642

Open in Web Open in Cursor 

Summary by cubic

Make the CLI update check non‑blocking by moving the network fetch to a detached background process. Commands now exit immediately even on slow/offline networks, addressing BU-642.

  • Bug Fixes
    • checkForUpdates() is now synchronous; uses cached state, shows a stale notice if it indicates a newer version, and spawns a background refresh when the cache is stale/missing.
    • Introduced spawnBackgroundRefresh() to run node -e detached with stdio: 'ignore' and .unref(); spawn failures are ignored so the main process always exits.
    • buildRefreshScript() fetches the latest GitHub release and writes update-state.json, writing the current version on any failure or invalid response to avoid repeated retries.
    • resolveNodePath() selects the correct Node binary across platforms.
    • src/cli.ts: call checkForUpdates() without awaiting or catching.
    • Tests: mock node:child_process spawn; add coverage for background refresh, buildRefreshScript, resolveNodePath, spawn failure handling, and stale notice behavior.

Written for commit 53e8cc1. Summary will update on new commits.

cursoragent and others added 2 commits April 9, 2026 17:56
When the update-state cache is stale or missing, the CLI now spawns a
detached child process to refresh the cache in the background instead of
awaiting a fetch() in the main process. This prevents commands from
appearing to hang for up to 5 seconds on slow or offline networks.

Key changes:
- checkForUpdates() is now synchronous; it reads cached state, shows
  any stale notice, and kicks off a detached node process to refresh
- buildRefreshScript() generates a self-contained Node script that
  fetches the latest version and writes update-state.json, writing
  the current version as fallback on any failure (prevents repeated
  retries on every command)
- spawnBackgroundRefresh() spawns the script detached with stdio
  ignored and unrefs the child so the main process exits immediately
- All functions converted to const arrow functions per coding standards
- Removed JSDoc comments per coding standards

Resolves BU-642

Co-authored-by: Bu Kinoshita <bukinoshita@users.noreply.github.com>
@bukinoshita
Copy link
Copy Markdown
Member Author

@cubic-dev-ai can you review?

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai bot commented Apr 9, 2026

@cubic-dev-ai can you review?

@bukinoshita I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/lib/update-check.ts">

<violation number="1" location="src/lib/update-check.ts:30">
P2: Falling back to `'node'` disables background update refresh for standalone binary installs that do not have Node in PATH.</violation>
</file>

<file name="src/cli.ts">

<violation number="1" location="src/cli.ts:189">
P2: Wrap `checkForUpdates()` in a local guard; it can still throw on malformed cached state and currently bubbles into the global error handler.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

if (/(?:^|[\\/])node(?:\.exe)?$/i.test(process.execPath)) {
return process.execPath;
}
return 'node';
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Falling back to 'node' disables background update refresh for standalone binary installs that do not have Node in PATH.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/update-check.ts, line 30:

<comment>Falling back to `'node'` disables background update refresh for standalone binary installs that do not have Node in PATH.</comment>

<file context>
@@ -1,38 +1,81 @@
+  if (/(?:^|[\\/])node(?:\.exe)?$/i.test(process.execPath)) {
+    return process.execPath;
+  }
+  return 'node';
+};
+
</file context>
Fix with Cubic

}

return checkForUpdates().catch(() => {});
checkForUpdates();
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Wrap checkForUpdates() in a local guard; it can still throw on malformed cached state and currently bubbles into the global error handler.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli.ts, line 189:

<comment>Wrap `checkForUpdates()` in a local guard; it can still throw on malformed cached state and currently bubbles into the global error handler.</comment>

<file context>
@@ -186,7 +186,7 @@ program
     }
 
-    return checkForUpdates().catch(() => {});
+    checkForUpdates();
   })
   .catch((err) => {
</file context>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants