From 169cca996833097a70ef1b56fa4b887ae3b65d8e Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:53:37 -0300 Subject: [PATCH 1/4] Share in-flight calls through a singleFlight helper runSetup already let a second caller join the setup in progress; the pattern moves into src/singleFlight.ts so ensureRunning, the shared-memory prompt and MCP serving can use the same one. No change in behaviour. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/singleFlight.test.ts | 69 ++++++++++++++++++++++++++++++ src/lifecycle.ts | 23 +++++----- src/singleFlight.ts | 38 ++++++++++++++++ 3 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/singleFlight.test.ts create mode 100644 src/singleFlight.ts diff --git a/src/__tests__/singleFlight.test.ts b/src/__tests__/singleFlight.test.ts new file mode 100644 index 0000000..045ec8b --- /dev/null +++ b/src/__tests__/singleFlight.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi } from 'vitest'; +import { singleFlight } from '../singleFlight'; + +function deferred(): { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +} { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe('singleFlight', () => { + it('joins a call already in flight instead of starting another', async () => { + const flight = singleFlight(); + const work = deferred(); + const start = vi.fn(() => work.promise); + const onJoin = vi.fn(); + + const first = flight.run(start, onJoin); + const second = flight.run(start, onJoin); + expect(flight.current).toBeDefined(); + work.resolve('done'); + + expect(await Promise.all([first, second])).toEqual(['done', 'done']); + expect(start).toHaveBeenCalledTimes(1); + expect(onJoin).toHaveBeenCalledTimes(1); + }); + + it('starts afresh once the call has resolved', async () => { + const flight = singleFlight(); + const start = vi.fn(() => Promise.resolve(1)); + + await flight.run(start); + expect(flight.current).toBeUndefined(); + await flight.run(start); + + expect(start).toHaveBeenCalledTimes(2); + }); + + it('starts afresh once the call has rejected', async () => { + const flight = singleFlight(); + const start = vi + .fn<() => Promise>() + .mockRejectedValueOnce(new Error('first')) + .mockResolvedValueOnce(2); + + await expect(flight.run(start)).rejects.toThrow('first'); + expect(flight.current).toBeUndefined(); + expect(await flight.run(start)).toBe(2); + }); + + it('hands a joiner the same rejection', async () => { + const flight = singleFlight(); + const work = deferred(); + + const first = flight.run(() => work.promise); + const second = flight.run(() => Promise.resolve(0)); + work.reject(new Error('failed')); + + await expect(first).rejects.toThrow('failed'); + await expect(second).rejects.toThrow('failed'); + }); +}); diff --git a/src/lifecycle.ts b/src/lifecycle.ts index c539271..8bbca03 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -70,6 +70,7 @@ import { isOnNfs } from './networkFileSystem'; import { logoutAll } from './session'; import { ensureStatmonitor } from './statmonitor'; import { allowAutoStart } from './autoStart'; +import { singleFlight } from './singleFlight'; import { readUnattendedSetupMarker, writeUnattendedSetupMarker } from './unattendedSetupMarker'; import { DATABASE_OUTCOME, @@ -227,20 +228,18 @@ function paused(): void { * say so instead of offering a Set Up GemDB button that would only join it. */ export function runSetup(extensionPath: string, trigger: Trigger): Promise { - if (setupInFlight) { - log('Setup is already under way; waiting for it to finish.'); - return setupInFlight; - } - setContext('gemdb.settingUp', true); - const run = runSetupOnce(extensionPath, trigger).finally(() => { - setupInFlight = undefined; - setContext('gemdb.settingUp', false); - }); - setupInFlight = run; - return run; + return setupFlight.run( + () => { + setContext('gemdb.settingUp', true); + return runSetupOnce(extensionPath, trigger).finally(() => { + setContext('gemdb.settingUp', false); + }); + }, + () => log('Setup is already under way; waiting for it to finish.'), + ); } -let setupInFlight: Promise | undefined; +const setupFlight = singleFlight(); async function runSetupOnce(extensionPath: string, trigger: Trigger): Promise { reportSetupStarted(trigger); diff --git a/src/singleFlight.ts b/src/singleFlight.ts new file mode 100644 index 0000000..67f6417 --- /dev/null +++ b/src/singleFlight.ts @@ -0,0 +1,38 @@ +/** + * At most one call of something in flight in this window, shared by everyone + * who asks while it runs. + * + * The first caller starts the work; a caller arriving before it settles joins + * it and gets the same result, rejection included, instead of starting a + * second copy. The slot empties when the work settles, so the next caller + * starts afresh. Only the caller that started the work runs `start`, so + * anything `start` reports — telemetry above all — is reported once. + * + * This is in-window only. Between windows, the locks in `lock.ts` decide. + */ +export interface SingleFlight { + /** Start `start` unless a call is already in flight; then join that one. */ + run(start: () => Promise, onJoin?: () => void): Promise; + /** The call in flight, if any. */ + readonly current: Promise | undefined; +} + +export function singleFlight(): SingleFlight { + let inFlight: Promise | undefined; + return { + run(start, onJoin) { + if (inFlight) { + onJoin?.(); + return inFlight; + } + const run = start().finally(() => { + if (inFlight === run) inFlight = undefined; + }); + inFlight = run; + return run; + }, + get current() { + return inFlight; + }, + }; +} From 4d602a7f6fc34583e946cb4c1a5e8dc27d1c20f2 Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:55:39 -0300 Subject: [PATCH 2/4] Bring GemDB up once when several callers ask at once Auto-start and a cell, two notebooks, or a cell and Start each ran the whole of ensureRunning at the same time: two startProcesses, two account checks, two Grail file-ins, two MCP installs and starts, and a databaseStarted for each. ensureRunning is now single-flight within a window. A caller arriving mid-flight joins the call in progress and gets its result; only the caller that started it reports, under its own trigger. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 ++ CLAUDE.md | 4 +- docs/telemetry.md | 4 +- src/__tests__/oneBringUp.test.ts | 166 +++++++++++++++++++++++++++++++ src/lifecycle.ts | 19 +++- 5 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/oneBringUp.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index df952bb..6e67ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 no setter. A method's receiver (`self`, or whatever the def calls it), a tuple's items, group rows and a saved stack's values cannot be changed, and VS Code does not offer Set Value on them. +### Fixed + +- **Starting GemDB from two places at once starts it once.** A cell run while the database was + starting on its own, cells in two notebooks, or a cell and **GemDB: Start** each ran the whole + start: two stone starts, two installs of Python support, two MCP servers. Now whatever asks while + a start is under way in the same window waits for that start and gets its result. + ## [1.6.0] - 2026-10-07 The engine is unchanged at GemStone 4.0.0.a4, but start with a fresh database: delete diff --git a/CLAUDE.md b/CLAUDE.md index 2e6fb3a..ab34dd1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,7 +49,9 @@ measured about when to ask for shared memory. whether the user pressed Start or just ran a notebook cell. It finishes any outstanding preparation, prompts for shared memory, starts the processes, files Grail in, and brings the MCP server up. New entry points that need a database -should call it rather than checking and asking. +should call it rather than checking and asking. Within a window it is +single-flight (`singleFlight.ts`): a second caller waits for the call already +in progress and gets its result, and only the first caller reports telemetry. ## CI diff --git a/docs/telemetry.md b/docs/telemetry.md index feaca80..e2932d5 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -167,7 +167,9 @@ sent. The database was brought up, or failed to come up. Sent only when something actually happened. Running a cell when the database is already up sends nothing, and a failure is sent once for each trigger until the database next -starts. +starts. A request that arrives while a start is already under way in the same +window waits for it and sends nothing, so one start is one event, carrying the +trigger of whatever began it. | Property / measure | Values | | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------- | diff --git a/src/__tests__/oneBringUp.test.ts b/src/__tests__/oneBringUp.test.ts new file mode 100644 index 0000000..b8b67c3 --- /dev/null +++ b/src/__tests__/oneBringUp.test.ts @@ -0,0 +1,166 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { __resetSettings } from '../__mocks__/vscode'; +import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; + +// `ensureRunning` is single-flight within a window: two notebooks and Start +// pressed together would otherwise each run the whole bring-up, with two stone +// starts, two Grail file-ins and two events. These tests hold the stone start +// open on a promise so the calls genuinely overlap, then check that one run +// served them all and that only the caller that started it reported. +vi.mock('../cli', () => ({ writeCliScripts: () => {}, ensureCliCurrent: () => true })); +// Creating the database account logs in over GCI; that is integration's. +vi.mock('../account', () => ({ ensureDatabaseAccount: () => {} })); + +const bundledGrailStamp = vi.fn(() => 'grail=0.1-1-gabc\n'); +const grailNeedsUpdate = vi.fn(() => false); +const fileInGrail = vi.fn(async () => {}); +vi.mock('../grail', () => ({ + grailLabel: () => 'grail 0.1', + grailNeedsUpdate: () => grailNeedsUpdate(), + stageGrail: () => {}, + fileInGrail: () => fileInGrail(), + bundledGrailStamp: () => bundledGrailStamp(), +})); + +const grailInstalled = vi.fn(() => true); +const isInstalled = vi.fn(() => true); +vi.mock('../paths', () => ({ + databaseExists: () => isInstalled(), + databasePath: () => '/db', + enginePath: () => (isInstalled() ? '/engine' : undefined), + grailInstalled: () => grailInstalled(), + grailPath: () => '/grail', + grailStagedOnDisk: () => isInstalled(), + mcpPath: () => '/mcp', +})); + +const ensureOsConfigured = vi.fn( + async (_extensionPath: string, _trigger: string): Promise => 'alreadyConfigured', +); +const isSharedMemoryConfigured = vi.fn(async () => true); +vi.mock('../osConfig', async (importOriginal) => { + const actual = await importOriginal(); + return { + OS_CONFIG_RESULT: actual.OS_CONFIG_RESULT, + osConfigAllowsStart: actual.osConfigAllowsStart, + ensureOsConfigured: (extensionPath: string, trigger: string) => + ensureOsConfigured(extensionPath, trigger), + isSharedMemoryConfigured: () => isSharedMemoryConfigured(), + }; +}); + +const findStone = vi.fn(() => true); +const findNetldi = vi.fn(() => true); +const startStone = vi.fn(async () => {}); +const startNetldi = vi.fn(async () => {}); +vi.mock('../processes', () => ({ + listProcesses: () => [], + findStone: () => findStone(), + findNetldi: () => findNetldi(), + startStone: () => startStone(), + startNetldi: () => startNetldi(), + isListening: () => true, + isRunning: () => true, + stopNetldi: async () => {}, + stopStone: async () => {}, +})); + +vi.mock('../platform', () => ({ isSupportedPlatform: () => true, setContext: () => {} })); +vi.mock('../autoStart', () => ({ + allowAutoStart: () => {}, + autoStartSuppressed: () => false, + initAutoStart: () => {}, + suppressAutoStart: () => {}, +})); + +const { ensureRunning } = await import('../lifecycle'); +const { TRIGGER, initTelemetry } = await import('../telemetry'); +const { initUnattendedSetupMarker } = await import('../unattendedSetupMarker'); + +/** A promise the test settles by hand, so a bring-up stays in progress until it chooses. */ +function deferred() { + let resolve!: () => void; + let reject!: (error: Error) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe('ensureRunning', () => { + beforeEach(() => { + __resetSettings(); + bundledGrailStamp.mockReturnValue('grail=0.1-1-gabc\n'); + grailNeedsUpdate.mockReturnValue(true); + grailInstalled.mockReturnValue(false); + isInstalled.mockReturnValue(true); + ensureOsConfigured.mockReset().mockResolvedValue('alreadyConfigured'); + isSharedMemoryConfigured.mockReset().mockResolvedValue(true); + fileInGrail.mockClear(); + findStone.mockReturnValue(false); + findNetldi.mockReturnValue(true); + startStone.mockReset(); + + const context = fakeExtensionContext(); + initTelemetry(context, false); + initUnattendedSetupMarker(context.globalStorageUri.fsPath); + }); + + describe('when called while a bring-up is in progress', () => { + it('serves every caller with one run, reported once under the first caller', async () => { + const stoneStart = deferred(); + startStone.mockImplementation(async () => { + await stoneStart.promise; + findStone.mockReturnValue(true); + }); + + const calls = [ + ensureRunning('/ext', TRIGGER.autoStart), + ensureRunning('/ext', TRIGGER.notebook), + ensureRunning('/ext', TRIGGER.startCommand), + ]; + stoneStart.resolve(); + + expect(await Promise.all(calls)).toEqual([true, true, true]); + expect(startStone).toHaveBeenCalledTimes(1); + expect(fileInGrail).toHaveBeenCalledTimes(1); + const events = eventsNamed('databaseStarted'); + expect(events).toHaveLength(1); + expect(events[0].properties).toMatchObject({ + trigger: 'autoStart', + outcome: 'started', + filedGrail: 'firstTime', + }); + }); + + it('fails every caller together when the run fails, reporting the failure once', async () => { + const stoneStart = deferred(); + startStone.mockImplementation(() => stoneStart.promise); + + const calls = [ + ensureRunning('/ext', TRIGGER.notebook), + ensureRunning('/ext', TRIGGER.notebook), + ensureRunning('/ext', TRIGGER.startCommand), + ]; + stoneStart.reject(new Error('stone would not start')); + + expect(await Promise.all(calls)).toEqual([false, false, false]); + expect(startStone).toHaveBeenCalledTimes(1); + const events = eventsNamed('databaseStarted'); + expect(events).toHaveLength(1); + expect(events[0].properties).toMatchObject({ trigger: 'notebook', outcome: 'startFailed' }); + }); + }); + + it('starts a new run for a call made after the previous one has settled', async () => { + startStone.mockImplementation(async () => {}); + ensureOsConfigured.mockResolvedValue('configured'); + + await ensureRunning('/ext', TRIGGER.notebook); + await ensureRunning('/ext', TRIGGER.startCommand); + + expect(ensureOsConfigured).toHaveBeenCalledTimes(2); + expect(startStone).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/lifecycle.ts b/src/lifecycle.ts index 8bbca03..56d5aa5 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -503,9 +503,26 @@ export async function start(extensionPath: string): Promise { * pointed at a new root path — and here the prompt is justified because the * user has asked for something that cannot happen without it. * + * At most one runs at a time in this window. Auto-start and a cell, two + * notebooks, or a cell and Start would otherwise each run the whole bring-up + * at once — two stone starts, two Grail file-ins, two MCP servers. A caller + * arriving while one is in progress joins it, from the shared-memory prompt to + * the last step, and gets its result. A joiner reports nothing: + * `databaseStarted` is sent once, by the caller that started the call, under + * that caller's trigger. + * * Returns true when the database is up and Python will run. */ -export async function ensureRunning(extensionPath: string, trigger: Trigger): Promise { +export function ensureRunning(extensionPath: string, trigger: Trigger): Promise { + return bringUp.run( + () => ensureRunningOnce(extensionPath, trigger), + () => log('GemDB is already starting; waiting for it.'), + ); +} + +const bringUp = singleFlight(); + +async function ensureRunningOnce(extensionPath: string, trigger: Trigger): Promise { const stopwatch = Stopwatch.start(); const failed = (outcome: Exclude): false => { reportDatabaseStarted(trigger, outcome, FILED_GRAIL.no, stopwatch.elapsedMs(), false); From 2283312f45dd90f0d0dd6916713cb9196b51b820 Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:58:12 -0300 Subject: [PATCH 3/4] Ask about shared memory once per window, and run its script once The first-run prompt and a cell's ensureRunning each opened a modal; VS Code queued the second, and answering both could run the sudo script twice. ensureOsConfigured is now single-flight, so a second caller takes the first one's answer and osConfigPrompted is sent once, under the first caller's trigger. The shared-memory script is single-flight too, whether the prompt or "GemDB: Configure Shared Memory" runs it. The command during a prompt waits for it and shows its usual message without a second terminal; a prompt during the command's script waits, checks again and returns without a modal. Only the caller that ran the script reports. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 + docs/telemetry.md | 6 +- src/__mocks__/vscode.ts | 18 +++- src/__tests__/oneOsPrompt.test.ts | 166 ++++++++++++++++++++++++++++++ src/osConfig.ts | 82 ++++++++++++--- 5 files changed, 258 insertions(+), 19 deletions(-) create mode 100644 src/__tests__/oneOsPrompt.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e67ea3..5490a9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 starting on its own, cells in two notebooks, or a cell and **GemDB: Start** each ran the whole start: two stone starts, two installs of Python support, two MCP servers. Now whatever asks while a start is under way in the same window waits for that start and gets its result. +- **GemDB asks about shared memory once, even when two things need it.** A cell run while the + first-run question about shared memory was still open queued a second copy of it, and answering + both could open two `sudo` terminals. Now the second waits for the first answer. Running + **GemDB: Configure Shared Memory** while that setup is already running waits for it instead of + opening another terminal. ## [1.6.0] - 2026-10-07 diff --git a/docs/telemetry.md b/docs/telemetry.md index e2932d5..476f219 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -154,7 +154,11 @@ the Database Running After Logout** while RemoveIPC was unset. The dialog comes back every time the database is needed until the user agrees, but a failure is sent once for each trigger, outcome and `missing` until setup works or the database next starts. Running either command is not deduped: every run is -sent. +sent. A request that arrives while the dialog, or the shared-memory setup it +started, is already under way in the same window waits for it and sends +nothing, so one dialog is one event, carrying the trigger of whatever opened +it. The same holds for **GemDB: Configure Shared Memory** run during that +setup, and for a dialog that would open while the command's setup is running. | Property | Values | | --------- | ----------------------------------------------------------------------------- | diff --git a/src/__mocks__/vscode.ts b/src/__mocks__/vscode.ts index d88502f..5b67d3f 100644 --- a/src/__mocks__/vscode.ts +++ b/src/__mocks__/vscode.ts @@ -251,7 +251,16 @@ export const window = { dispose(): void; } { __terminals.push(nameOrOptions as FakeTerminalOptions); - return { show: () => {}, sendText: () => {}, dispose: () => {} }; + const terminal = { show: () => {}, sendText: () => {}, dispose: () => {} }; + __openTerminals.push(terminal); + return terminal; + }, + /** Fires when a test calls `__closeTerminal`, as the editor does when the user closes one. */ + onDidCloseTerminal(listener: (terminal: unknown) => void): Disposable { + __closeTerminalListeners.push(listener); + return new Disposable(() => { + __closeTerminalListeners.splice(__closeTerminalListeners.indexOf(listener), 1); + }); }, /** * Progress is a pass-through here: run the callback and return its result. @@ -489,6 +498,13 @@ export interface FakeTerminalOptions { }; } export const __terminals: FakeTerminalOptions[] = []; +/** The terminal objects `createTerminal` handed back, oldest first. */ +export const __openTerminals: object[] = []; +const __closeTerminalListeners: Array<(terminal: unknown) => void> = []; +/** Close a terminal the way the editor reports it to `onDidCloseTerminal`. */ +export function __closeTerminal(terminal: object): void { + for (const listener of [...__closeTerminalListeners]) listener(terminal); +} export const workspace = { notebookDocuments: [] as unknown[], diff --git a/src/__tests__/oneOsPrompt.test.ts b/src/__tests__/oneOsPrompt.test.ts new file mode 100644 index 0000000..89671a1 --- /dev/null +++ b/src/__tests__/oneOsPrompt.test.ts @@ -0,0 +1,166 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as vscode from '../__mocks__/vscode'; +import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; + +/** + * The vscode-facing wrappers are one at a time in a window: the first-run + * prompt, a cell's, and "GemDB: Configure Shared Memory" must not each open a + * modal or a `sudo` terminal. They live apart from osConfig.test.ts because + * they need the real wrappers, with the operating system faked underneath. + */ + +// The limits `isSharedMemoryConfigured` reads, as macOS's sysctl prints them. +let enoughMemory = false; +vi.mock('child_process', async (original) => ({ + ...(await original()), + execFile: ( + _file: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => { + const gb = enoughMemory ? 1024 : 0.004; + const bytes = Math.round(gb * 2 ** 30); + callback(null, `kern.sysv.shmmax: ${bytes}\nkern.sysv.shmall: ${Math.round(bytes / 4096)}\n`); + }, +})); + +const { ensureOsConfigured, configureSharedMemory } = await import('../osConfig'); +const { TRIGGER, initTelemetry } = await import('../telemetry'); + +const originalPlatform = process.platform; +const sent = (): Record[] => + eventsNamed('osConfigPrompted').map((e) => e.properties as Record); + +let answer: (choice: string | undefined) => void; +let modal: ReturnType; +let information: ReturnType; +let error: ReturnType; + +/** The user finishing the terminal: the script has run, so memory is as given. */ +function finishScript(options: { took: boolean }): void { + enoughMemory = options.took; + vscode.__closeTerminal(vscode.__openTerminals[0]); +} + +beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + enoughMemory = false; + vscode.__resetSettings(); + vscode.__terminals.length = 0; + vscode.__openTerminals.length = 0; + initTelemetry(fakeExtensionContext(), false); + modal = vi.spyOn(vscode.window, 'showWarningMessage').mockImplementation( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + information = vi.spyOn(vscode.window, 'showInformationMessage'); + error = vi.spyOn(vscode.window, 'showErrorMessage'); +}); + +afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + vi.restoreAllMocks(); +}); + +describe('one OS prompt at a time', () => { + it('shares one modal, one terminal and one event between callers with different triggers', async () => { + const first = ensureOsConfigured('/ext', TRIGGER.firstRun); + const second = ensureOsConfigured('/ext', TRIGGER.notebook); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + + answer('Configure'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + finishScript({ took: true }); + + expect(await first).toBe('configured'); + expect(await second).toBe('configured'); + expect(modal).toHaveBeenCalledTimes(1); + expect(vscode.__terminals).toHaveLength(1); + expect(sent()).toHaveLength(1); + expect(sent()[0]).toMatchObject({ trigger: 'firstRun', outcome: 'configured' }); + }); + + it('lets the command join a prompt whose script is running, without a second terminal or event', async () => { + const prompt = ensureOsConfigured('/ext', TRIGGER.firstRun); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + answer('Configure'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + + const command = configureSharedMemory('/ext'); + // Let the command reach its wait before the terminal closes. + await new Promise((resolve) => setTimeout(resolve, 0)); + finishScript({ took: true }); + await command; + + expect(await prompt).toBe('configured'); + expect(vscode.__terminals).toHaveLength(1); + expect(sent()).toHaveLength(1); + expect(sent()[0]).toMatchObject({ trigger: 'firstRun' }); + expect(information).toHaveBeenCalledWith('Shared memory configured.'); + expect(error).not.toHaveBeenCalled(); + }); + + it('lets the command join a prompt still waiting on its modal', async () => { + const prompt = ensureOsConfigured('/ext', TRIGGER.firstRun); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + + const command = configureSharedMemory('/ext'); + await new Promise((resolve) => setTimeout(resolve, 0)); + answer('Configure'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + finishScript({ took: false }); + await command; + + expect(await prompt).toBe('stillUnconfigured'); + expect(vscode.__terminals).toHaveLength(1); + expect(error).toHaveBeenCalledWith(expect.stringContaining('still below')); + expect(sent()).toHaveLength(1); + }); + + it("waits for the command's script instead of asking, and reports nothing", async () => { + const command = configureSharedMemory('/ext'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + + const prompt = ensureOsConfigured('/ext', TRIGGER.notebook); + await new Promise((resolve) => setTimeout(resolve, 0)); + finishScript({ took: true }); + + expect(await prompt).toBe('configured'); + await command; + expect(modal).not.toHaveBeenCalled(); + expect(vscode.__terminals).toHaveLength(1); + // Only the command's own report: the waiting prompt sends none. + expect(sent()).toHaveLength(1); + expect(sent()[0]).toMatchObject({ trigger: 'sharedMemoryCommand', outcome: 'configured' }); + }); + + it("answers stillUnconfigured when the command's script did not take", async () => { + const command = configureSharedMemory('/ext'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + + const prompt = ensureOsConfigured('/ext', TRIGGER.notebook); + await new Promise((resolve) => setTimeout(resolve, 0)); + finishScript({ took: false }); + + expect(await prompt).toBe('stillUnconfigured'); + await command; + expect(modal).not.toHaveBeenCalled(); + expect(sent()).toHaveLength(1); + expect(sent()[0]).toMatchObject({ trigger: 'sharedMemoryCommand' }); + }); + + it('starts afresh once the first prompt has settled', async () => { + const first = ensureOsConfigured('/ext', TRIGGER.firstRun); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + answer(undefined); + expect(await first).toBe('declined'); + + const second = ensureOsConfigured('/ext', TRIGGER.notebook); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(2)); + answer(undefined); + expect(await second).toBe('declined'); + }); +}); diff --git a/src/osConfig.ts b/src/osConfig.ts index 55e7c3f..13ed846 100644 --- a/src/osConfig.ts +++ b/src/osConfig.ts @@ -4,6 +4,7 @@ import { execFile } from 'child_process'; import * as vscode from 'vscode'; import { REQUIRED_SHARED_MEMORY_GB } from './config'; import { log } from './log'; +import { singleFlight } from './singleFlight'; import { OS_CONFIG_MISSING, OS_CONFIG_OUTCOME, @@ -342,26 +343,45 @@ export async function runEnsureOsConfigured(world: OsConfigWorld): Promise { + return promptFlight.run( + () => ensureOsConfiguredOnce(extensionPath, trigger), + () => log('GemDB is already asking to configure shared memory; waiting for the answer.'), + ); +} + +const promptFlight = singleFlight(); + +async function ensureOsConfiguredOnce( + extensionPath: string, + trigger: Trigger, +): Promise { + const scriptRunning = sharedMemoryScriptFlight.current; + if (scriptRunning) { + log('Shared memory is already being configured; waiting for it.'); + await scriptRunning; + return (await isSharedMemoryConfigured()) + ? OS_CONFIG_RESULT.configured + : OS_CONFIG_RESULT.stillUnconfigured; + } return runEnsureOsConfigured({ sharedMemoryOk: isSharedMemoryConfigured, removeIpcOk: isRemoveIpcConfigured, confirm: async (message) => (await vscode.window.showWarningMessage(message, { modal: true }, 'Configure')) === 'Configure', - runSharedMemoryScript: () => - runSetupScript( - SHARED_MEMORY_TERMINAL, - path.join( - extensionPath, - 'resources', - process.platform === 'linux' ? 'setSharedMemoryLinux.sh' : 'setSharedMemoryDarwin.sh', - ), - ), + runSharedMemoryScript: () => runSharedMemoryScript(extensionPath), runRemoveIpcScript: () => runSetupScript(REMOVE_IPC_TERMINAL, path.join(extensionPath, 'resources', 'setRemoveIPC.sh')), showError: (message) => void vscode.window.showErrorMessage(message), @@ -377,17 +397,21 @@ export function ensureOsConfigured( * else tells the user whether it worked. Reported as `osConfigPrompted` when * shared memory was actually short, because this is the path back after * declining the modal, and without it that recovery is invisible. + * + * When the prompt, or the script it opened, is already under way in this + * window, this waits for it and says how it went instead of opening a second + * terminal; the prompt reports that run, so this reports nothing. */ export async function configureSharedMemory(extensionPath: string): Promise { const wasShort = !(await isSharedMemoryConfigured()); - await runSetupScript( - SHARED_MEMORY_TERMINAL, - path.join( - extensionPath, - 'resources', - process.platform === 'linux' ? 'setSharedMemoryLinux.sh' : 'setSharedMemoryDarwin.sh', - ), - ); + const inFlight = promptFlight.current ?? sharedMemoryScriptFlight.current; + if (inFlight) { + log('Shared memory is already being configured; waiting for it.'); + await inFlight.catch(() => undefined); + showSharedMemoryResult(await isSharedMemoryConfigured()); + return; + } + await runSharedMemoryScript(extensionPath); const configured = await isSharedMemoryConfigured(); if (wasShort) { reportOsConfigPrompted( @@ -396,6 +420,10 @@ export async function configureSharedMemory(extensionPath: string): Promise { + return sharedMemoryScriptFlight.run(() => + runSetupScript( + SHARED_MEMORY_TERMINAL, + path.join( + extensionPath, + 'resources', + process.platform === 'linux' ? 'setSharedMemoryLinux.sh' : 'setSharedMemoryDarwin.sh', + ), + ), + ); +} + +const sharedMemoryScriptFlight = singleFlight(); + /** * Open the RemoveIPC setup on its own, from the status view's "Survives * logout" row. From fb953b88d7fa877e92a9abd893e373a082ffd897 Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:58:32 -0300 Subject: [PATCH 4/4] Install and start the MCP server once per window resumeMcpServing at activation and ensureRunning both reach ensureMcpServing, and on a running stone they could install the MCP classes twice and race to fork two routers onto one port. ensureMcpServing is now single-flight; a second caller waits for the first and gets its result. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++ src/__tests__/oneBringUp.test.ts | 59 ++++++++++++++++++++++++++++++-- src/lifecycle.ts | 17 +++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5490a9a..038e2d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 both could open two `sudo` terminals. Now the second waits for the first answer. Running **GemDB: Configure Shared Memory** while that setup is already running waits for it instead of opening another terminal. +- **The MCP server is installed and started once when GemDB starts.** Opening a window on a running + database while a cell started it could install the MCP server's classes twice and start two + servers on one port. Now the second waits for the first. ## [1.6.0] - 2026-10-07 diff --git a/src/__tests__/oneBringUp.test.ts b/src/__tests__/oneBringUp.test.ts index b8b67c3..29ced3c 100644 --- a/src/__tests__/oneBringUp.test.ts +++ b/src/__tests__/oneBringUp.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { __resetSettings } from '../__mocks__/vscode'; +import { __resetSettings, __setSetting } from '../__mocks__/vscode'; import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; // `ensureRunning` is single-flight within a window: two notebooks and Start @@ -65,6 +65,22 @@ vi.mock('../processes', () => ({ stopStone: async () => {}, })); +// The MCP server is off unless a test turns `gemdb.mcp.enabled` on. +const mcpNeedsUpdate = vi.fn(() => false); +const installMcp = vi.fn(async () => {}); +const isMcpRunning = vi.fn(async () => false); +const startMcpServer = vi.fn(async () => true); +vi.mock('../mcp', async (importOriginal) => ({ + ...(await importOriginal()), + bundledMcpStamp: () => 'mcp=abc\n', + mcpNeedsUpdate: () => mcpNeedsUpdate(), + stageMcp: () => {}, + installMcp: () => installMcp(), + recordMcpInstalled: () => {}, + isMcpRunning: () => isMcpRunning(), + startMcpServer: () => startMcpServer(), +})); + vi.mock('../platform', () => ({ isSupportedPlatform: () => true, setContext: () => {} })); vi.mock('../autoStart', () => ({ allowAutoStart: () => {}, @@ -73,7 +89,7 @@ vi.mock('../autoStart', () => ({ suppressAutoStart: () => {}, })); -const { ensureRunning } = await import('../lifecycle'); +const { ensureRunning, resumeMcpServing } = await import('../lifecycle'); const { TRIGGER, initTelemetry } = await import('../telemetry'); const { initUnattendedSetupMarker } = await import('../unattendedSetupMarker'); @@ -164,3 +180,42 @@ describe('ensureRunning', () => { expect(startStone).toHaveBeenCalledTimes(2); }); }); + +// Activation's `resumeMcpServing` and a cell's `ensureRunning` both reach +// `ensureMcpServing`; on a running stone they would otherwise install the +// classes twice and fork two routers at one port. +describe('ensureMcpServing', () => { + beforeEach(() => { + __resetSettings(); + __setSetting('gemdb.mcp.enabled', true); + grailNeedsUpdate.mockReturnValue(false); + grailInstalled.mockReturnValue(true); + isInstalled.mockReturnValue(true); + ensureOsConfigured.mockReset().mockResolvedValue('alreadyConfigured'); + findStone.mockReturnValue(true); + findNetldi.mockReturnValue(true); + mcpNeedsUpdate.mockReturnValue(true); + installMcp.mockReset(); + isMcpRunning.mockReset().mockResolvedValue(false); + startMcpServer.mockReset().mockResolvedValue(true); + + const context = fakeExtensionContext(); + initTelemetry(context, false); + initUnattendedSetupMarker(context.globalStorageUri.fsPath); + }); + + it('installs and starts the MCP server once when resume and a start overlap', async () => { + const install = deferred(); + installMcp.mockImplementation(() => install.promise); + + const resumed = resumeMcpServing('/ext'); + const started = ensureRunning('/ext', TRIGGER.notebook); + // Let both callers reach the MCP step before the install finishes. + await new Promise((resolve) => setTimeout(resolve, 20)); + install.resolve(); + + expect(await Promise.all([resumed, started])).toEqual([true, true]); + expect(installMcp).toHaveBeenCalledTimes(1); + expect(startMcpServer).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lifecycle.ts b/src/lifecycle.ts index 56d5aa5..10beec0 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -656,6 +656,12 @@ function grailFileInDue(extensionPath: string): boolean { * Installing is separate from running, as with Grail: the classes are filed * into the database once per payload build, and the router is forked whenever * one is not already listening. + * + * At most one runs at a time in this window. `ensureRunning` and activation's + * `resumeMcpServing` both come here, and two at once would install the + * classes twice and race to fork two routers onto one port. A caller arriving + * while one is in progress waits for it and gets its result; its progress + * notification says nothing about the MCP server meanwhile. */ async function ensureMcpServing( extensionPath: string, @@ -668,7 +674,18 @@ async function ensureMcpServing( log('This build of GemDB ships no MCP server payload, so there is none to run.'); return false; } + return mcpServing.run( + () => ensureMcpServingOnce(extensionPath, progress), + () => log('The MCP server is already starting; waiting for it.'), + ); +} + +const mcpServing = singleFlight(); +async function ensureMcpServingOnce( + extensionPath: string, + progress?: vscode.Progress<{ message?: string }>, +): Promise { try { if (mcpNeedsUpdate(extensionPath)) { const stamp = bundledMcpStamp(extensionPath);