From 9f3d7137675a56d6c4c6ec2eae69ae435108c809 Mon Sep 17 00:00:00 2001 From: Daniel Martin Date: Sun, 16 Aug 2026 14:51:42 +0200 Subject: [PATCH] fix(connect): don't quit a Figma that is already debuggable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connect` quit and relaunched Figma unconditionally, costing the user their window arrangement and any unsaved state — including the common case where the CDP port already answers and nothing needs to happen. connect now probes the port first and branches on the result: reuse CDP answers -> leave Figma alone, wire up the daemon needs-quit Figma up, no debug port -> ask the user to quit (they may have unsaved work; we must not kill it) start-fresh no Figma -> patch if needed and launch, as before The decision itself is a pure function in src/lib/connect-plan.js so it can be unit-tested without a Figma or an open port. The process probe uses isFigmaRunning() from platform.js, so it works on all three platforms. Co-Authored-By: Claude Opus 5 --- src/commands/setup.js | 65 ++++++++++++++++++++++++++------------ src/lib/connect-plan.js | 30 ++++++++++++++++++ tests/connect-plan.test.js | 21 ++++++++++++ 3 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 src/lib/connect-plan.js create mode 100644 tests/connect-plan.test.js diff --git a/src/commands/setup.js b/src/commands/setup.js index 4a4757d1..19c3eed9 100644 --- a/src/commands/setup.js +++ b/src/commands/setup.js @@ -8,7 +8,8 @@ import { join, basename } from 'path'; import { FigmaClient } from '../figma-client.js'; import * as apiDocs from '../api-docs.js'; import { isPatched, patchFigma, unpatchFigma, getCdpPort } from '../figma-patch.js'; -import { detectBrowser, startBrowserApp, getBrowserCommand } from '../platform.js'; +import { detectBrowser, startBrowserApp, getBrowserCommand, isFigmaRunning } from '../platform.js'; +import { resolveConnectAction } from '../lib/connect-plan.js'; import { convert, detectSourceType } from '../code-import/index.js'; import { program, @@ -750,32 +751,54 @@ program // Stop any existing daemon stopDaemon(); - console.log(chalk.blue('Starting Figma...')); + // Don't touch a Figma that is already debuggable. Quitting it costs the + // user their window arrangement and any unsaved state for nothing. + let cdpReachable = false; try { - killFigma(); - await new Promise(r => setTimeout(r, 500)); + const probe = await fetch(`http://localhost:${getCdpPort()}/json`, { signal: AbortSignal.timeout(2000) }); + cdpReachable = probe.ok; } catch {} - startFigma(); - console.log(chalk.green('✓ Figma started\n')); + const action = resolveConnectAction({ cdpReachable, figmaRunning: isFigmaRunning() }); - // Wait and check connection - const spinner = ora('Waiting for connection...').start(); - let connected = false; - for (let i = 0; i < 8; i++) { - await new Promise(r => setTimeout(r, 1000)); - const result = figmaUse('status', { silent: true }); - if (result && result.includes('Connected')) { - spinner.succeed('Connected to Figma'); - console.log(chalk.gray(result.trim())); - connected = true; - break; - } + if (action === 'needs-quit') { + // Figma is up but without --remote-debugging-port. Only the user can + // quit it safely, so ask rather than kill. + console.log(chalk.yellow('\n Figma is running, but the debug port is not open.')); + console.log(chalk.white(' Quit Figma (Cmd+Q), then run ') + chalk.cyan('connect') + chalk.white(' again.\n')); + return; } - if (!connected) { - spinner.warn('Open a file in Figma to connect'); - return; + if (action === 'start-fresh') { + console.log(chalk.blue('Starting Figma...')); + try { + killFigma(); + await new Promise(r => setTimeout(r, 500)); + } catch {} + + startFigma(); + console.log(chalk.green('✓ Figma started\n')); + + // Wait and check connection + const spinner = ora('Waiting for connection...').start(); + let connected = false; + for (let i = 0; i < 8; i++) { + await new Promise(r => setTimeout(r, 1000)); + const result = figmaUse('status', { silent: true }); + if (result && result.includes('Connected')) { + spinner.succeed('Connected to Figma'); + console.log(chalk.gray(result.trim())); + connected = true; + break; + } + } + + if (!connected) { + spinner.warn('Open a file in Figma to connect'); + return; + } + } else { + console.log(chalk.green('✓ Figma already running (left untouched)\n')); } // Start daemon for fast commands (force restart to get fresh connection) diff --git a/src/lib/connect-plan.js b/src/lib/connect-plan.js new file mode 100644 index 00000000..3b88293f --- /dev/null +++ b/src/lib/connect-plan.js @@ -0,0 +1,30 @@ +/** + * Deciding what `connect` has to do to Figma before it can talk to it. + * + * Lives in its own module so the decision can be unit-tested: the connect + * command itself probes the CDP port and the process list, neither of which + * is available in a test run. + */ + +/** + * What `connect` should do, given what is currently running. + * + * `connect` used to quit and relaunch Figma unconditionally. That costs the + * user their window arrangement and any unsaved state every time they run it — + * including the common case where Figma is already reachable and nothing needs + * to happen to it at all. + * + * @param {object} state + * @param {boolean} state.cdpReachable the CDP port answered + * @param {boolean} state.figmaRunning a Figma process exists + * @returns {'reuse'|'needs-quit'|'start-fresh'} + * `reuse` — Figma is already debuggable; leave it alone, just wire up the daemon. + * `needs-quit` — Figma runs without the debug port. Only the user can quit it + * safely (unsaved work), so ask instead of killing it. + * `start-fresh` — no Figma at all; patch if needed and launch it ourselves. + */ +export function resolveConnectAction({ cdpReachable, figmaRunning }) { + if (cdpReachable) return 'reuse'; + if (figmaRunning) return 'needs-quit'; + return 'start-fresh'; +} diff --git a/tests/connect-plan.test.js b/tests/connect-plan.test.js new file mode 100644 index 00000000..c4c5c21c --- /dev/null +++ b/tests/connect-plan.test.js @@ -0,0 +1,21 @@ +// Unit tests for the connect decision (pure, no Figma/CDP needed). +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { resolveConnectAction } from '../src/lib/connect-plan.js'; + +test('a reachable CDP port means Figma is left alone', () => { + assert.equal(resolveConnectAction({ cdpReachable: true, figmaRunning: true }), 'reuse'); +}); + +test('CDP wins even if the process probe missed Figma', () => { + // A reachable port proves a debuggable Figma exists, whatever pgrep says. + assert.equal(resolveConnectAction({ cdpReachable: true, figmaRunning: false }), 'reuse'); +}); + +test('Figma running without the debug port asks the user to quit', () => { + assert.equal(resolveConnectAction({ cdpReachable: false, figmaRunning: true }), 'needs-quit'); +}); + +test('no Figma at all means we start it ourselves', () => { + assert.equal(resolveConnectAction({ cdpReachable: false, figmaRunning: false }), 'start-fresh'); +});