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/8] 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/8] 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/8] 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/8] 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); From e327e02e878406c734311a6731c8f46d82c85f5a Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:29:13 -0300 Subject: [PATCH 5/8] Release held flights after each single-flight test The prompt, the shared-memory script, the bring-up and the MCP step are module-level single flights, and nothing reset them between tests. A test that failed with a modal unanswered, a terminal open or a deferred unresolved handed its flight to every later test, which joined it and timed out, so one failure read as many. oneOsPrompt.test.ts now cancels any pending modal and closes every terminal (a new __closeAllTerminals in the vscode mock) after each test, for a few rounds so a follow-on step gets drained too, before the spies are restored. oneBringUp.test.ts records every deferred it hands out and resolves them all after each test. Co-Authored-By: Claude Opus 5.5 --- src/__mocks__/vscode.ts | 7 +++++++ src/__tests__/oneBringUp.test.ts | 14 +++++++++++++- src/__tests__/oneOsPrompt.test.ts | 24 +++++++++++++++++------- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/__mocks__/vscode.ts b/src/__mocks__/vscode.ts index 5b67d3f..bee747c 100644 --- a/src/__mocks__/vscode.ts +++ b/src/__mocks__/vscode.ts @@ -505,6 +505,13 @@ const __closeTerminalListeners: Array<(terminal: unknown) => void> = []; export function __closeTerminal(terminal: object): void { for (const listener of [...__closeTerminalListeners]) listener(terminal); } +/** + * Close every terminal still open, so a test that failed half way does not + * leave a setup script waiting for the next test to finish it. + */ +export function __closeAllTerminals(): void { + for (const terminal of __openTerminals.splice(0)) __closeTerminal(terminal); +} export const workspace = { notebookDocuments: [] as unknown[], diff --git a/src/__tests__/oneBringUp.test.ts b/src/__tests__/oneBringUp.test.ts index 29ced3c..d61add3 100644 --- a/src/__tests__/oneBringUp.test.ts +++ b/src/__tests__/oneBringUp.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { __resetSettings, __setSetting } from '../__mocks__/vscode'; import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; @@ -93,6 +93,9 @@ const { ensureRunning, resumeMcpServing } = await import('../lifecycle'); const { TRIGGER, initTelemetry } = await import('../telemetry'); const { initUnattendedSetupMarker } = await import('../unattendedSetupMarker'); +/** Every deferred a test made, so `afterEach` can release whatever a failed test left waiting. */ +const deferreds: Array<{ resolve: () => void }> = []; + /** A promise the test settles by hand, so a bring-up stays in progress until it chooses. */ function deferred() { let resolve!: () => void; @@ -101,9 +104,18 @@ function deferred() { resolve = res; reject = rej; }); + deferreds.push({ resolve }); return { promise, resolve, reject }; } +// The bring-up and the MCP step are module-level single flights. A test that +// fails with one still held open would hand it to every later test, which +// would join it and time out — so release every deferred and let it drain. +afterEach(async () => { + for (const d of deferreds.splice(0)) d.resolve(); + for (let round = 0; round < 5; round++) await new Promise((resolve) => setTimeout(resolve, 0)); +}); + describe('ensureRunning', () => { beforeEach(() => { __resetSettings(); diff --git a/src/__tests__/oneOsPrompt.test.ts b/src/__tests__/oneOsPrompt.test.ts index 89671a1..3312986 100644 --- a/src/__tests__/oneOsPrompt.test.ts +++ b/src/__tests__/oneOsPrompt.test.ts @@ -32,7 +32,7 @@ const originalPlatform = process.platform; const sent = (): Record[] => eventsNamed('osConfigPrompted').map((e) => e.properties as Record); -let answer: (choice: string | undefined) => void; +let answer: ((choice: string | undefined) => void) | undefined; let modal: ReturnType; let information: ReturnType; let error: ReturnType; @@ -50,6 +50,7 @@ beforeEach(() => { vscode.__terminals.length = 0; vscode.__openTerminals.length = 0; initTelemetry(fakeExtensionContext(), false); + answer = undefined; modal = vi.spyOn(vscode.window, 'showWarningMessage').mockImplementation( () => new Promise((resolve) => { @@ -60,7 +61,16 @@ beforeEach(() => { error = vi.spyOn(vscode.window, 'showErrorMessage'); }); -afterEach(() => { +// The prompt and the script are module-level single flights. A test that fails +// with one still open would hand it to every later test, which would join it +// and time out, so settle whatever is left — cancel the modal, close the +// terminals — before the spies are restored. +afterEach(async () => { + for (let round = 0; round < 5; round++) { + answer?.(undefined); + vscode.__closeAllTerminals(); + await new Promise((resolve) => setTimeout(resolve, 0)); + } Object.defineProperty(process, 'platform', { value: originalPlatform }); vi.restoreAllMocks(); }); @@ -71,7 +81,7 @@ describe('one OS prompt at a time', () => { const second = ensureOsConfigured('/ext', TRIGGER.notebook); await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); - answer('Configure'); + answer?.('Configure'); await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); finishScript({ took: true }); @@ -86,7 +96,7 @@ describe('one OS prompt at a time', () => { 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'); + answer?.('Configure'); await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); const command = configureSharedMemory('/ext'); @@ -109,7 +119,7 @@ describe('one OS prompt at a time', () => { const command = configureSharedMemory('/ext'); await new Promise((resolve) => setTimeout(resolve, 0)); - answer('Configure'); + answer?.('Configure'); await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); finishScript({ took: false }); await command; @@ -155,12 +165,12 @@ describe('one OS prompt at a time', () => { 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); + answer?.(undefined); expect(await first).toBe('declined'); const second = ensureOsConfigured('/ext', TRIGGER.notebook); await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(2)); - answer(undefined); + answer?.(undefined); expect(await second).toBe('declined'); }); }); From 3f316d672d1c43b3817a16cd1af028b3ba93caed Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:32:03 -0300 Subject: [PATCH 6/8] Share only the shared-memory script between the prompt and the command The prompt and "GemDB: Configure Shared Memory" coordinated by peeking at each other's single flight: a prompt that saw the command's script running waited for it instead of probing and asking, and the command joined the whole prompt when one was under way. Both shortcuts skipped steps. Now each runs its normal sequence and they share only the script. The prompt always probes, shows the modal, starts or joins the script, then offers RemoveIPC. The command probes, starts or joins the script, probes again and reports, and never looks at the prompt. This closes the gaps the shortcuts opened: - a cell no longer waits on the command's sudo terminal when memory is already enough; it answers alreadyConfigured at once - the command runs its script even when the modal it overlapped is cancelled - a prompt that joins the command's script still offers RemoveIPC on Linux - the command no longer waits on the RemoveIPC terminal after its script - a cell that showed no modal no longer answers configured, which counted as GemDB having prompted - the command's first probe is used again rather than wasted The command now always sends its own osConfigPrompted event when memory was short, as on main, including when it joined the prompt's script. A modal can appear while the command's terminal is open, but "Configure" joins that terminal, so there is still one sudo prompt. SingleFlight loses its `current` getter, which nothing reads any more. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/oneOsPrompt.test.ts | 187 ++++++++++++++++++++++------- src/__tests__/singleFlight.test.ts | 3 - src/osConfig.ts | 49 +++----- src/singleFlight.ts | 5 - 4 files changed, 162 insertions(+), 82 deletions(-) diff --git a/src/__tests__/oneOsPrompt.test.ts b/src/__tests__/oneOsPrompt.test.ts index 3312986..c336238 100644 --- a/src/__tests__/oneOsPrompt.test.ts +++ b/src/__tests__/oneOsPrompt.test.ts @@ -1,3 +1,4 @@ +import { promises as fsPromises } from 'fs'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as vscode from '../__mocks__/vscode'; import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; @@ -9,18 +10,24 @@ import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; * they need the real wrappers, with the operating system faked underneath. */ -// The limits `isSharedMemoryConfigured` reads, as macOS's sysctl prints them. +// The limits `isSharedMemoryConfigured` reads — as macOS's sysctl prints them, +// or as Linux's /proc holds them — and what `busctl` says about RemoveIPC. let enoughMemory = false; +let removeIpcOff = true; +const sharedMemoryBytes = (): number => Math.round((enoughMemory ? 1024 : 0.004) * 2 ** 30); vi.mock('child_process', async (original) => ({ ...(await original()), execFile: ( - _file: string, + 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); + if (file === 'busctl') { + callback(null, `b ${removeIpcOff ? 'false' : 'true'}\n`); + return; + } + const bytes = sharedMemoryBytes(); callback(null, `kern.sysv.shmmax: ${bytes}\nkern.sysv.shmall: ${Math.round(bytes / 4096)}\n`); }, })); @@ -28,6 +35,9 @@ vi.mock('child_process', async (original) => ({ const { ensureOsConfigured, configureSharedMemory } = await import('../osConfig'); const { TRIGGER, initTelemetry } = await import('../telemetry'); +const SHARED_MEMORY_TERMINAL = 'GemDB: Shared Memory Setup'; +const REMOVE_IPC_TERMINAL = 'GemDB: RemoveIPC Setup'; + const originalPlatform = process.platform; const sent = (): Record[] => eventsNamed('osConfigPrompted').map((e) => e.properties as Record); @@ -37,15 +47,39 @@ let modal: ReturnType; let information: ReturnType; let error: ReturnType; -/** The user finishing the terminal: the script has run, so memory is as given. */ +/** Terminals opened so far with this name, in order. */ +const terminalsNamed = (name: string): number => + vscode.__terminals.filter((t) => (t as unknown) === name).length; + +/** Close the most recent terminal with this name, as the user finishing it. */ +function closeTerminal(name: string): void { + const index = (vscode.__terminals as unknown[]).lastIndexOf(name); + vscode.__closeTerminal(vscode.__openTerminals[index]); +} + +/** The user finishing the shared-memory terminal: the script has run, so memory is as given. */ function finishScript(options: { took: boolean }): void { enoughMemory = options.took; - vscode.__closeTerminal(vscode.__openTerminals[0]); + closeTerminal(SHARED_MEMORY_TERMINAL); +} + +/** Let every pending promise chain run, so a test can assert that something is still waiting. */ +const settle = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)); + +function onLinux(): void { + Object.defineProperty(process, 'platform', { value: 'linux' }); + vi.spyOn(fsPromises, 'readFile').mockImplementation((async (file: unknown) => { + if (String(file).startsWith('/proc/sys/kernel/shmmax')) return String(sharedMemoryBytes()); + if (String(file).startsWith('/proc/sys/kernel/shmall')) + return String(Math.round(sharedMemoryBytes() / 4096)); + throw new Error(`unexpected read of ${String(file)}`); + }) as typeof fsPromises.readFile); } beforeEach(() => { Object.defineProperty(process, 'platform', { value: 'darwin' }); enoughMemory = false; + removeIpcOff = true; vscode.__resetSettings(); vscode.__terminals.length = 0; vscode.__openTerminals.length = 0; @@ -69,7 +103,7 @@ afterEach(async () => { for (let round = 0; round < 5; round++) { answer?.(undefined); vscode.__closeAllTerminals(); - await new Promise((resolve) => setTimeout(resolve, 0)); + await settle(); } Object.defineProperty(process, 'platform', { value: originalPlatform }); vi.restoreAllMocks(); @@ -93,84 +127,151 @@ describe('one OS prompt at a time', () => { 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 () => { + 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'); + }); +}); + +describe('the command and the prompt share one script, and nothing else', () => { + it('lets the command join the script a prompt started, without a second terminal', 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)); + await settle(); 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' }); + // One event each: the modal's answer, and the command run while memory was short. + expect(sent()).toEqual([ + expect.objectContaining({ trigger: 'firstRun', outcome: 'configured' }), + expect.objectContaining({ trigger: 'sharedMemoryCommand', outcome: 'configured' }), + ]); + expect(information).toHaveBeenCalledTimes(1); 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); + it('runs the script for the command even when the modal it overlapped is cancelled', async () => { + const prompt = ensureOsConfigured('/ext', TRIGGER.notebook); 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 }); + answer?.(undefined); + expect(await prompt).toBe('declined'); + + finishScript({ took: true }); await command; + expect(vscode.__terminals).toHaveLength(1); + expect(information).toHaveBeenCalledWith('Shared memory configured.'); + expect(error).not.toHaveBeenCalled(); + }); + + it('lets a modal answered "Configure" join the script the command already opened', async () => { + const command = configureSharedMemory('/ext'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + + // Raising shared memory always asks, even with the command's terminal open. + const prompt = ensureOsConfigured('/ext', TRIGGER.notebook); + await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + answer?.('Configure'); + await settle(); + expect(vscode.__terminals).toHaveLength(1); + finishScript({ took: false }); expect(await prompt).toBe('stillUnconfigured'); + await command; expect(vscode.__terminals).toHaveLength(1); - expect(error).toHaveBeenCalledWith(expect.stringContaining('still below')); - expect(sent()).toHaveLength(1); + expect(sent()).toEqual([ + expect.objectContaining({ trigger: 'sharedMemoryCommand', outcome: 'stillUnconfigured' }), + expect.objectContaining({ trigger: 'notebook', outcome: 'stillUnconfigured' }), + ]); }); - it("waits for the command's script instead of asking, and reports nothing", async () => { + it("answers alreadyConfigured at once when memory is enough, even with the command's terminal open", async () => { + enoughMemory = true; 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)); + expect(await ensureOsConfigured('/ext', TRIGGER.notebook)).toBe('alreadyConfigured'); + expect(modal).not.toHaveBeenCalled(); + // Memory was never short, so neither the prompt nor the command reports. finishScript({ took: true }); + await command; + expect(sent()).toHaveLength(0); + }); - expect(await prompt).toBe('configured'); + it('reports nothing for a cell whose modal was never shown', async () => { + enoughMemory = true; + const command = configureSharedMemory('/ext'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); + + // The cell overlaps the command's script but shows no modal, so it must + // not answer `configured`: that would count as GemDB having prompted. + const prompt = ensureOsConfigured('/ext', TRIGGER.notebook); + finishScript({ took: true }); await command; + expect(await prompt).toBe('alreadyConfigured'); 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' }); + expect(sent()).toHaveLength(0); + }); +}); + +describe('on Linux, with RemoveIPC unset', () => { + beforeEach(() => { + onLinux(); + removeIpcOff = false; }); - it("answers stillUnconfigured when the command's script did not take", async () => { + it("still offers RemoveIPC to a prompt that joined the command's script", 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 vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); + answer?.('Configure'); + await settle(); + finishScript({ took: true }); await command; - expect(modal).not.toHaveBeenCalled(); - expect(sent()).toHaveLength(1); - expect(sent()[0]).toMatchObject({ trigger: 'sharedMemoryCommand' }); + + await vi.waitFor(() => expect(terminalsNamed(REMOVE_IPC_TERMINAL)).toBe(1)); + closeTerminal(REMOVE_IPC_TERMINAL); + expect(await prompt).toBe('removeIpcUnset'); + expect(terminalsNamed(SHARED_MEMORY_TERMINAL)).toBe(1); }); - it('starts afresh once the first prompt has settled', async () => { - const first = ensureOsConfigured('/ext', TRIGGER.firstRun); + it('lets the command finish without waiting on the RemoveIPC terminal that follows', async () => { + const prompt = ensureOsConfigured('/ext', TRIGGER.firstRun); await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(1)); - answer?.(undefined); - expect(await first).toBe('declined'); + answer?.('Configure'); + await vi.waitFor(() => expect(vscode.__openTerminals).toHaveLength(1)); - const second = ensureOsConfigured('/ext', TRIGGER.notebook); - await vi.waitFor(() => expect(modal).toHaveBeenCalledTimes(2)); - answer?.(undefined); - expect(await second).toBe('declined'); + let commandDone = false; + const command = configureSharedMemory('/ext').then(() => { + commandDone = true; + }); + await settle(); + finishScript({ took: true }); + await vi.waitFor(() => expect(terminalsNamed(REMOVE_IPC_TERMINAL)).toBe(1)); + await settle(); + + expect(commandDone).toBe(true); + await command; + expect(information).toHaveBeenCalledWith('Shared memory configured.'); + closeTerminal(REMOVE_IPC_TERMINAL); + expect(await prompt).toBe('removeIpcUnset'); }); }); diff --git a/src/__tests__/singleFlight.test.ts b/src/__tests__/singleFlight.test.ts index 045ec8b..d60fd2a 100644 --- a/src/__tests__/singleFlight.test.ts +++ b/src/__tests__/singleFlight.test.ts @@ -24,7 +24,6 @@ describe('singleFlight', () => { 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']); @@ -37,7 +36,6 @@ describe('singleFlight', () => { const start = vi.fn(() => Promise.resolve(1)); await flight.run(start); - expect(flight.current).toBeUndefined(); await flight.run(start); expect(start).toHaveBeenCalledTimes(2); @@ -51,7 +49,6 @@ describe('singleFlight', () => { .mockResolvedValueOnce(2); await expect(flight.run(start)).rejects.toThrow('first'); - expect(flight.current).toBeUndefined(); expect(await flight.run(start)).toBe(2); }); diff --git a/src/osConfig.ts b/src/osConfig.ts index 13ed846..24325f5 100644 --- a/src/osConfig.ts +++ b/src/osConfig.ts @@ -349,7 +349,9 @@ export async function runEnsureOsConfigured(world: OsConfigWorld): 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, @@ -398,19 +392,14 @@ async function ensureOsConfiguredOnce( * 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. + * When the script is already running in this window — the prompt opened it, + * or this command did a moment ago — this joins it instead of opening a second + * terminal, then checks and reports as usual. It never waits on the prompt + * itself: not on a modal nobody has answered, nor on the RemoveIPC step that + * follows the script. */ export async function configureSharedMemory(extensionPath: string): Promise { const wasShort = !(await isSharedMemoryConfigured()); - 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) { @@ -420,10 +409,6 @@ 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', + return sharedMemoryScriptFlight.run( + () => + runSetupScript( + SHARED_MEMORY_TERMINAL, + path.join( + extensionPath, + 'resources', + process.platform === 'linux' ? 'setSharedMemoryLinux.sh' : 'setSharedMemoryDarwin.sh', + ), ), - ), + () => log('The shared-memory setup is already open in a terminal; waiting for it.'), ); } diff --git a/src/singleFlight.ts b/src/singleFlight.ts index 67f6417..32026a7 100644 --- a/src/singleFlight.ts +++ b/src/singleFlight.ts @@ -13,8 +13,6 @@ 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 { @@ -31,8 +29,5 @@ export function singleFlight(): SingleFlight { inFlight = run; return run; }, - get current() { - return inFlight; - }, }; } From 9e4417c9f6acd79a5611fc8341430dae291f51db Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:34:48 -0300 Subject: [PATCH 7/8] Start again for a caller that arrives after a Stop on a bring-up Stop takes no lock, so it can land on a bring-up already under way. A cell run or Start pressed after that Stop joined the window's bring-up and took its result, which described a database Stop had since taken down: the stone stayed stopped, the cell failed at login or met "GemDB is not running", and the joiner never ran allowAutoStart, so auto-start stayed suppressed. On main that caller ran a bring-up of its own and started the stone again. stop() now counts the Stops begun in this window and keeps the one under way; the bring-up's owner records the count when it starts. A caller that joins with a different count, meaning a Stop began after the bring-up did, waits for the shared result (true or false), waits for that Stop, and runs ensureRunning again. A Stop pressed after the caller joined still wins. Each rerun needs another Stop, so this cannot loop. No process check is involved: isRunning blocks the extension host. allowAutoStart moves from the bring-up body into ensureRunning, so a joiner retracts an earlier Stop too. Auto-start is the exception: it is GemDB's own doing, and calling it there would undo a Stop pressed while auto-start waits on another window's setup. A Stop cancelled at its "Stop Anyway" modal is harmless: a caller that joined after it waits for the modal, then reruns, finds the stone up, and only brings back the MCP server Stop had already taken down. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/oneBringUp.test.ts | 173 ++++++++++++++++++++++++++++++- src/lifecycle.ts | 59 +++++++++-- 2 files changed, 220 insertions(+), 12 deletions(-) diff --git a/src/__tests__/oneBringUp.test.ts b/src/__tests__/oneBringUp.test.ts index d61add3..a8ddf55 100644 --- a/src/__tests__/oneBringUp.test.ts +++ b/src/__tests__/oneBringUp.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as vscode from 'vscode'; import { __resetSettings, __setSetting } from '../__mocks__/vscode'; import { eventsNamed, fakeExtensionContext } from './telemetryTestSupport'; @@ -53,6 +54,9 @@ const findStone = vi.fn(() => true); const findNetldi = vi.fn(() => true); const startStone = vi.fn(async () => {}); const startNetldi = vi.fn(async () => {}); +const stopStone = vi.fn(async (_force: boolean) => { + findStone.mockReturnValue(false); +}); vi.mock('../processes', () => ({ listProcesses: () => [], findStone: () => findStone(), @@ -60,9 +64,9 @@ vi.mock('../processes', () => ({ startStone: () => startStone(), startNetldi: () => startNetldi(), isListening: () => true, - isRunning: () => true, + isRunning: () => findStone(), stopNetldi: async () => {}, - stopStone: async () => {}, + stopStone: (force: boolean) => stopStone(force), })); // The MCP server is off unless a test turns `gemdb.mcp.enabled` on. @@ -79,17 +83,19 @@ vi.mock('../mcp', async (importOriginal) => ({ recordMcpInstalled: () => {}, isMcpRunning: () => isMcpRunning(), startMcpServer: () => startMcpServer(), + stopMcpServer: async () => {}, })); vi.mock('../platform', () => ({ isSupportedPlatform: () => true, setContext: () => {} })); +const allowAutoStart = vi.fn(); vi.mock('../autoStart', () => ({ - allowAutoStart: () => {}, + allowAutoStart: () => allowAutoStart(), autoStartSuppressed: () => false, initAutoStart: () => {}, suppressAutoStart: () => {}, })); -const { ensureRunning, resumeMcpServing } = await import('../lifecycle'); +const { ensureRunning, resumeMcpServing, stop } = await import('../lifecycle'); const { TRIGGER, initTelemetry } = await import('../telemetry'); const { initUnattendedSetupMarker } = await import('../unattendedSetupMarker'); @@ -231,3 +237,162 @@ describe('ensureMcpServing', () => { expect(startMcpServer).toHaveBeenCalledTimes(1); }); }); + +// Stop takes no lock, so it can land on a bring-up under way. A caller who +// arrives after that Stop is asking for the database back: it must not be +// handed a result that described a database Stop has since taken down. +describe('ensureRunning after a Stop', () => { + beforeEach(() => { + __resetSettings(); + __setSetting('gemdb.mcp.enabled', true); + grailNeedsUpdate.mockReturnValue(false); + grailInstalled.mockReturnValue(true); + isInstalled.mockReturnValue(true); + ensureOsConfigured.mockReset().mockResolvedValue('alreadyConfigured'); + fileInGrail.mockReset(); + findStone.mockReturnValue(false); + findNetldi.mockReturnValue(true); + startStone.mockReset().mockImplementation(async () => { + findStone.mockReturnValue(true); + }); + stopStone.mockClear(); + mcpNeedsUpdate.mockReturnValue(true); + installMcp.mockReset(); + isMcpRunning.mockReset().mockResolvedValue(false); + startMcpServer.mockReset().mockResolvedValue(true); + allowAutoStart.mockClear(); + + const context = fakeExtensionContext(); + initTelemetry(context, false); + initUnattendedSetupMarker(context.globalStorageUri.fsPath); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('starts the database again for a cell run after Stop, while auto-start is in the MCP step', async () => { + const install = deferred(); + installMcp.mockImplementationOnce(() => install.promise); + + const auto = ensureRunning('/ext', TRIGGER.autoStart); + await vi.waitFor(() => expect(installMcp).toHaveBeenCalledTimes(1)); + await stop(); + expect(findStone()).toBeFalsy(); + + const cell = ensureRunning('/ext', TRIGGER.notebook); + install.resolve(); + + expect(await auto).toBe(true); + expect(await cell).toBe(true); + expect(findStone()).toBe(true); + expect(startStone).toHaveBeenCalledTimes(2); + // The cell retracted the Stop, once per pass; auto-start, GemDB's own doing, did not. + expect(allowAutoStart).toHaveBeenCalledTimes(2); + }); + + it('starts again for a cell run after Stop even when the bring-up it joined failed', async () => { + // Stop lands during the Grail file-in, which then fails for want of a stone. + grailNeedsUpdate.mockReturnValue(true); + grailInstalled.mockReturnValue(false); + const fileIn = deferred(); + fileInGrail.mockImplementationOnce(() => fileIn.promise); + + const auto = ensureRunning('/ext', TRIGGER.autoStart); + await vi.waitFor(() => expect(fileInGrail).toHaveBeenCalledTimes(1)); + await stop(); + const cell = ensureRunning('/ext', TRIGGER.notebook); + fileIn.reject(new Error('no stone')); + + expect(await auto).toBe(false); + expect(await cell).toBe(true); + expect(findStone()).toBe(true); + expect(fileInGrail).toHaveBeenCalledTimes(2); + }); + + it('joins without starting again when no Stop came between', async () => { + const install = deferred(); + installMcp.mockImplementationOnce(() => install.promise); + + const first = ensureRunning('/ext', TRIGGER.notebook); + await vi.waitFor(() => expect(installMcp).toHaveBeenCalledTimes(1)); + const second = ensureRunning('/ext', TRIGGER.startCommand); + install.resolve(); + + expect(await Promise.all([first, second])).toEqual([true, true]); + expect(startStone).toHaveBeenCalledTimes(1); + expect(allowAutoStart).toHaveBeenCalledTimes(2); + }); + + it('lets a Stop pressed after the cell joined win', async () => { + const install = deferred(); + installMcp.mockImplementationOnce(() => install.promise); + + const auto = ensureRunning('/ext', TRIGGER.autoStart); + await vi.waitFor(() => expect(installMcp).toHaveBeenCalledTimes(1)); + const cell = ensureRunning('/ext', TRIGGER.notebook); + await stop(); + install.resolve(); + + await auto; + await cell; + expect(startStone).toHaveBeenCalledTimes(1); + expect(findStone()).toBeFalsy(); + }); + + it('lets a Stop stand for an auto-start that joins after it', async () => { + const install = deferred(); + installMcp.mockImplementationOnce(() => install.promise); + + const cell = ensureRunning('/ext', TRIGGER.notebook); + await vi.waitFor(() => expect(installMcp).toHaveBeenCalledTimes(1)); + await stop(); + const auto = ensureRunning('/ext', TRIGGER.autoStart); + install.resolve(); + + await cell; + await auto; + expect(startStone).toHaveBeenCalledTimes(1); + expect(findStone()).toBeFalsy(); + }); + + it('leaves the database up when the Stop the cell came after is cancelled at its modal', async () => { + const install = deferred(); + installMcp.mockImplementationOnce(() => install.promise); + // A session holds the stone up, so Stop asks before forcing it; the user + // dismisses the modal, which is the same answer as Cancel. + stopStone.mockImplementationOnce(async () => { + throw new Error('sessions are still logged in'); + }); + const modal = deferred(); + const warning = vi + .spyOn(vscode.window, 'showWarningMessage') + .mockImplementationOnce(() => modal.promise.then(() => undefined)); + + const auto = ensureRunning('/ext', TRIGGER.autoStart); + await vi.waitFor(() => expect(installMcp).toHaveBeenCalledTimes(1)); + const stopping = stop(); + await vi.waitFor(() => expect(warning).toHaveBeenCalledTimes(1)); + + let cellDone = false; + const cell = ensureRunning('/ext', TRIGGER.notebook).then((running) => { + cellDone = true; + return running; + }); + install.resolve(); + expect(await auto).toBe(true); + // The cell goes after the Stop it followed, so it waits on that modal. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(cellDone).toBe(false); + + modal.resolve(); + await stopping; + expect(await cell).toBe(true); + // Nothing was stopped, so nothing was started again: the second pass + // found the stone up and only brought back the MCP server Stop had taken down. + expect(stopStone).toHaveBeenCalledTimes(1); + expect(startStone).toHaveBeenCalledTimes(1); + expect(startMcpServer).toHaveBeenCalledTimes(2); + expect(findStone()).toBe(true); + }); +}); diff --git a/src/lifecycle.ts b/src/lifecycle.ts index 10beec0..7d90eba 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -511,16 +511,52 @@ export async function start(extensionPath: string): Promise { * `databaseStarted` is sent once, by the caller that started the call, under * that caller's trigger. * + * Except after a Stop: a caller other than auto-start that arrives once Stop + * has been pressed on the bring-up under way is asking for the database back, so it waits for that + * bring-up and the Stop to finish and then starts again, rather than taking a + * result that described a database Stop has since taken down. A Stop pressed + * after the caller joined wins: it was pressed later, so it is the latest word. + * * Returns true when the database is up and Python will run. */ -export function ensureRunning(extensionPath: string, trigger: Trigger): Promise { - return bringUp.run( - () => ensureRunningOnce(extensionPath, trigger), - () => log('GemDB is already starting; waiting for it.'), +export async function ensureRunning(extensionPath: string, trigger: Trigger): Promise { + // Asking for a running database is the clearest possible retraction of an + // earlier "stop it". Running a cell counts: `ensureRunning` is the one path + // to a running database, so it is the one place this belongs — here rather + // than in the bring-up, so that a caller who joins one counts too. Auto-start + // is GemDB's own doing, not a request from anybody, so it retracts nothing. + if (trigger !== TRIGGER.autoStart) allowAutoStart(); + let stoppedBeforeJoining = false; + const running = await bringUp.run( + () => { + stopsWhenBringUpBegan = stopsBegun; + return ensureRunningOnce(extensionPath, trigger); + }, + () => { + stoppedBeforeJoining = stopsBegun !== stopsWhenBringUpBegan; + log('GemDB is already starting; waiting for it.'); + }, ); + // Whatever the shared result, true or false, it predates this caller's + // request. Each rerun needs another Stop to have begun, so this cannot loop. + // Auto-start asks for nothing, so it takes the result and lets the Stop stand. + if (!stoppedBeforeJoining || trigger === TRIGGER.autoStart) return running; + await stopInFlight; + log('GemDB was stopped while it was starting; starting it again.'); + return ensureRunning(extensionPath, trigger); } const bringUp = singleFlight(); +/** + * How many times `stop` has begun in this window, and that count when the + * bring-up in flight began. Counted rather than asked of the processes: + * `isRunning` blocks the extension host, and a stone that is up says nothing + * about whether a Stop is on its way to it. + */ +let stopsBegun = 0; +let stopsWhenBringUpBegan = 0; +/** The Stop under way, if any, so a caller starting again goes after it. */ +let stopInFlight: Promise | undefined; async function ensureRunningOnce(extensionPath: string, trigger: Trigger): Promise { const stopwatch = Stopwatch.start(); @@ -529,10 +565,6 @@ async function ensureRunningOnce(extensionPath: string, trigger: Trigger): Promi return false; }; - // Asking for a running database is the clearest possible retraction of an - // earlier "stop it". Running a cell counts: `ensureRunning` is the one path - // to a running database, so it is the one place this belongs. - allowAutoStart(); if (!requireSupportedPlatform()) return failed(DATABASE_OUTCOME.unsupportedPlatform); if (!requireGrailPayload(extensionPath)) return failed(DATABASE_OUTCOME.missingPayload); @@ -920,6 +952,17 @@ export async function stop(): Promise { ); return; } + stopsBegun++; + const stopping = stopOnce(); + stopInFlight = stopping; + try { + await stopping; + } finally { + if (stopInFlight === stopping) stopInFlight = undefined; + } +} + +async function stopOnce(): Promise { await vscode.window.withProgress( { location: vscode.ProgressLocation.Notification, title: 'Stopping GemDB' }, async () => { From ee7146292ee30a1b6b8dacb20a9a01af7fa04af9 Mon Sep 17 00:00:00 2001 From: MatiasFernandez <7898112+MatiasFernandez@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:36:48 -0300 Subject: [PATCH 8/8] Describe the shared-memory and Stop exceptions in the docs The telemetry doc said the command waited for the prompt's setup and sent nothing; it now shares the script's terminal and sends its own event, and a dialog that opens while the command's setup runs is shown and sent as usual. databaseStarted, the changelog and CLAUDE.md now say that a caller arriving after a Stop on the bring-up starts again. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 ++++--- CLAUDE.md | 4 +++- docs/telemetry.md | 19 ++++++++++++------- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 038e2d1..827dbab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,12 +35,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **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. + a start is under way in the same window waits for that start and gets its result, unless the + database was stopped while it started: then it waits for the stop and starts the database again. - **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. + **GemDB: Configure Shared Memory** while that setup is already running uses its terminal instead + of opening another, and so does answering the question while the command's setup is running. - **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. diff --git a/CLAUDE.md b/CLAUDE.md index ab34dd1..cbdf434 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,9 @@ 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. 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. +in progress and gets its result, and only the first caller reports telemetry — +except after a Stop: a caller other than auto-start arriving once a Stop began +during that bring-up waits for the Stop and starts again. ## CI diff --git a/docs/telemetry.md b/docs/telemetry.md index 476f219..2a9fa9f 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -153,12 +153,15 @@ Memory** while shared memory was still too low, or when they ran **GemDB: Keep 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. 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. +database next starts. That rule does not apply to either command: every run +made while the setting was still unchanged is 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 command is not such a request: +run during the dialog's setup, **GemDB: Configure Shared Memory** shares its +terminal but sends its own event, and a dialog that comes up while the +command's setup is running is shown and sent as usual, its "Configure" joining +that terminal rather than opening a second one. | Property | Values | | --------- | ----------------------------------------------------------------------------- | @@ -173,7 +176,9 @@ 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. 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. +trigger of whatever began it. The exception is a request that arrives after +the user stopped the database during that start: it waits for the stop and +then asks again, so the start that follows is an event of its own. | Property / measure | Values | | ------------------ | ----------------------------------------------------------------------------------------------------------------------------------- |