From 6f7288ff19086a416d6682ecf7ae70c50af145af Mon Sep 17 00:00:00 2001 From: Jose Antonio Alatorre Date: Sun, 25 Oct 2026 12:53:07 +0100 Subject: [PATCH] Bound the catalog refresh, keep unavailable connections in the file, scope listings before a refresh (0.3.14) Two defects found while accepting the 0.7.9 Codex release: - A forced catalog refresh took 773 s and wrote the scope empty without naming the connection unavailable. readProjectToolCatalog now runs within one budget (budgetMs, default DEFAULT_PROJECT_REFRESH_BUDGET_MS = 300 s): a connection whose read fails, or is not read before the budget runs out, stays in the catalog marked `unavailable: { at, code, message }` with its earlier tools, so a scope whose connections exist is never written empty and healthy. The mark lives in the file, so state() and the mcp-catalog action report it from any process, not only the one that ran the refresh; a successful read or update() clears it. A refresh aborts its dispatcher's signal when it returns, so a read the budget left behind cannot keep a socket. - A fresh Codex server's first tools/list listed every cached scope because the backend was learned only from a refresh. createProjectToolCatalog takes `backend` and offers ready(), which resolves it without a network request the way auth-status does (environment, shared preferences, default); until the backend is known a listing is empty, never another backend's tools. Tests cover the written mark and its clearing, the budget (hung reads, retained tools, released signal), the empty-until-known listing and ready(); the mcp-catalog test checks that the next process sees the mark. ADR 0004 and the README record the behaviour. Co-Authored-By: Claude Fable 5.1 --- README.md | 29 ++-- docs/adr/0004-shared-project-tool-registry.md | 25 +++- package-lock.json | 4 +- package.json | 2 +- src/index.ts | 4 +- src/shared/projectTools.ts | 129 ++++++++++++------ tests/project-tools.test.cjs | 78 +++++++++-- 7 files changed, 196 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index 1018dda..aadc915 100644 --- a/README.md +++ b/README.md @@ -228,18 +228,25 @@ is specific to their host. content blocks, then one JSON text block with `summary`, `effects`, `error` and `structuredContent` when the call failed, returned structured content or no blocks; `isError` on any non-success, with the remote content of an `MCP_TOOL_ERROR` preserved. -- `readProjectToolCatalog(dispatch, { backend?, environmentUid?, previous?, signal? })`: - `mcp-list` (read-only), then one `mcp-tools` per saved connection, two at a time. A - connection whose catalog cannot be read keeps its previous tools and is named in - `unavailable`; a removed connection loses them; connections outside the scope are - carried over. The session for a connection's backend is verified when its tool is called. -- `createProjectToolCatalog({ dispatch, file?, staleMs?, notify?, disabled? })`: the shared - on-disk catalog, `project-tool-catalog.json` beside `mcp-connections.json` +- `readProjectToolCatalog(dispatch, { backend?, environmentUid?, previous?, signal?, budgetMs? })`: + `mcp-list` (read-only), then one `mcp-tools` per saved connection, two at a time, all + within one budget (300 s by default). A connection whose catalog cannot be read, or is + not read before the budget runs out, keeps its previous tools and stays in the catalog + marked `unavailable` with the reason, so a scope whose connections exist is never + written empty and healthy; a removed connection loses its tools; connections outside the + scope are carried over. The session for a connection's backend is verified when its tool + is called. +- `createProjectToolCatalog({ dispatch, file?, backend?, budgetMs?, staleMs?, notify?, disabled? })`: + the shared on-disk catalog, `project-tool-catalog.json` beside `mcp-connections.json` (`mainSequenceStateDirectory(env)` names that directory). `list()` answers from the file - at once, scoped to the current account's backend once a refresh has learned it; - `refresh()` re-reads under a lock, so one process reads catalogs at a time while the - others pick its result up from the file; `update()`, `dispose()` and `notify` serve a - long-lived host such as the Codex MCP server. The file holds schemas and labels only. + at once, for the host's backend only: the one given, or resolved by `ready()` without a + network request the way `auth-status` resolves it, or learned from a refresh; until it is + known a listing is empty, never another backend's tools. `state()` reports the connections + marked `unavailable` in the file, whichever process marked them. `refresh()` re-reads + under a lock, so one process reads catalogs at a time while the others pick its result up + from the file, and ends its dispatcher's signal when it returns; `update()`, `dispose()` + and `notify` serve a long-lived host such as the Codex MCP server. The file holds schemas + and labels only. - The `mcp-catalog` action returns the same definitions, bindings and state for a host that dispatches actions instead of importing: without `refresh` from the file, with `refresh: true` re-reading a stale cache, with `'force'` always, waiting up to two diff --git a/docs/adr/0004-shared-project-tool-registry.md b/docs/adr/0004-shared-project-tool-registry.md index 2684b18..f5f46cd 100644 --- a/docs/adr/0004-shared-project-tool-registry.md +++ b/docs/adr/0004-shared-project-tool-registry.md @@ -94,7 +94,13 @@ Additive to the existing `createDevTools()` surface; nothing existing changes sh whose catalog cannot be read keeps its `previous` tools and is listed as `unavailable`; a removed connection loses its tools. No `expectedAccount` snapshot is attached: a registered tool is bound to a saved connection, and the session for that - connection's backend is verified when the tool is called. + connection's backend is verified when the tool is called. Since 0.3.14 the whole read + runs within one budget (`budgetMs`, default 300 s, `DEFAULT_PROJECT_REFRESH_BUDGET_MS`): + a connection not read by then, like one whose read failed, stays in the catalog marked + `unavailable: { at, code, message }` with its earlier tools, so a scope whose + connections exist is never written empty and healthy, and the mark is in the file, where + a host whose process did not run the refresh still sees it. The first release wrote an + empty scope after a 773 s refresh and reported nothing; this is the correction. - `createProjectToolCatalog({ file?, dispatch, staleMs?, notify?, disabled? })`: the on-disk catalog (default `codex-tool-catalog.json` is renamed to a host-neutral `project-tool-catalog.json` beside `mcp-connections.json`), with atomic writes, a lock @@ -104,12 +110,17 @@ Additive to the existing `createDevTools()` surface; nothing existing changes sh host's change announcement. It stores schemas and labels only; never credentials, tool results or tokens. Because the file is shared by every host on the machine, a refresh replaces only the connections in its scope (the backend, and the environment - when given) and carries the rest over, and `list()`/`state()` are scoped to the current - account's backend once a refresh has learned it from `auth-status`, so another - backend's cached connections stay out of a host's tool list. `refresh(force, { scope?, - waitMs? })` resolves to whether this process read and wrote the catalog; with `waitMs` - it waits for another process's lock instead of skipping, and reports `PROJECT_BUSY` - in its state when the wait runs out. + when given) and carries the rest over, and `list()`/`state()` are scoped to the host's + backend: the one given as `backend`, or resolved by `ready()` without any network + request the way `auth-status` resolves it (process environment, shared preferences, + default), or learned from a refresh. Until the backend is known a listing is empty, + never another backend's tools (the first release listed every cached scope on a fresh + server's first `tools/list`; a host now awaits `ready()` before answering it). + `refresh(force, { scope?, waitMs? })` resolves to whether this process read and wrote + the catalog; with `waitMs` it waits for another process's lock instead of skipping, and + reports `PROJECT_BUSY` in its state when the wait runs out. A refresh ends its + dispatcher's signal when it returns, so a read the budget left behind cannot keep a + socket or the lock. - `mainSequenceStateDirectory(env)`: the one place that knows `~/Library/Application Support/MainSequenceCLI`, `%APPDATA%\MainSequenceCLI` and `~/.config/mainsequence`, used by the connection store and the catalog. diff --git a/package-lock.json b/package-lock.json index 52f4e34..f72b0e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@dev-mainsequence/devtools", - "version": "0.3.13", + "version": "0.3.14", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@dev-mainsequence/devtools", - "version": "0.3.13", + "version": "0.3.14", "license": "MIT", "bin": { "ms-devtools": "dist/runner.js" diff --git a/package.json b/package.json index eded788..1836da1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@dev-mainsequence/devtools", - "version": "0.3.13", + "version": "0.3.14", "description": "Shared MainSequence developer actions, project operations, MCP client and command runner", "license": "MIT", "engines": { diff --git a/src/index.ts b/src/index.ts index 2c92fe9..d25d9c8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -33,9 +33,9 @@ export type { ActionData, DiscoveryResultMap, AgentDirectory, McpConnection, Mcp export { mainSequenceStateDirectory } from './shared/stateDirectory'; export { projectToolName, isProjectToolName, projectToolConnection, projectToolConnectionLabel, projectToolDefinition, projectToolDefinitions, projectToolBinding, projectToolRegistration, projectToolResult, projectToolTimeoutMs, readProjectToolCatalog, readProjectToolCatalogFile, projectToolCatalogPath, createProjectToolCatalog, - PROJECT_TOOL_PREFIX, MAX_PROJECT_TOOLS_PER_CONNECTION, PROJECT_TOOL_TIMEOUT_HINT, DEFAULT_PROJECT_REQUEST_TIMEOUT_MS, MAX_PROJECT_TOOL_TIMEOUT_MS } from './shared/projectTools'; + PROJECT_TOOL_PREFIX, MAX_PROJECT_TOOLS_PER_CONNECTION, PROJECT_TOOL_TIMEOUT_HINT, DEFAULT_PROJECT_REQUEST_TIMEOUT_MS, MAX_PROJECT_TOOL_TIMEOUT_MS, DEFAULT_PROJECT_REFRESH_BUDGET_MS } from './shared/projectTools'; export type { ProjectToolBinding, ProjectToolConnection, ProjectToolCatalog, ProjectToolCatalogOptions, ProjectToolCatalogState, ProjectToolCatalogStore, ProjectToolCatalogView, - ProjectToolDefinition, ProjectToolDispatch, ProjectToolResult, ProjectToolScope } from './shared/projectTools'; + ProjectToolDefinition, ProjectToolDispatch, ProjectToolResult, ProjectToolScope, ProjectToolUnavailable } from './shared/projectTools'; export { listEnvironments } from './shared/environments'; export type { EnvironmentSummary } from './shared/environments'; export { bindSessionAccount } from './shared/accountScope'; diff --git a/src/shared/projectTools.ts b/src/shared/projectTools.ts index b9776ee..d227f62 100644 --- a/src/shared/projectTools.ts +++ b/src/shared/projectTools.ts @@ -5,6 +5,7 @@ import { randomUUID } from 'node:crypto'; import { ActionRequest, ActionResult, ErrorCode, SessionStatus, ToolingError } from './models'; import type { McpCatalog, McpConnection, McpTool } from './discoveryTypes'; import { mainSequenceStateDirectory } from './stateDirectory'; +import { resolveBackend } from './repositories'; /** * Saved project MCP tools as first-class tools of a host (ADR 0004). One tool name, one @@ -17,7 +18,10 @@ export type ProjectToolDispatch = (request: ActionRequest) => Promise; }; export type ProjectToolCatalogOptions = { - /** A dispatcher for one refresh; the signal aborts when the catalog is disposed mid-refresh. */ + /** A dispatcher for one refresh; the signal aborts when the refresh ends or the catalog is disposed. */ dispatch: (signal: AbortSignal) => ProjectToolDispatch; file?: string; env?: NodeJS.ProcessEnv; staleMs?: number; disabled?: boolean; + /** The backend whose connections this host lists; otherwise `ready()` resolves it the way `auth-status` does. */ + backend?: string; + /** How long one refresh may take in all; connections not read by then are unavailable. */ + budgetMs?: number; /** Called when the registered definitions changed, for a host's change announcement. */ notify?: () => void; }; @@ -47,6 +55,9 @@ export const MAX_PROJECT_TOOLS_PER_CONNECTION = 80; export const PROJECT_TOOL_TIMEOUT_HINT = 'mainsequence.ai/timeout-seconds/v1'; export const DEFAULT_PROJECT_REQUEST_TIMEOUT_MS = 120_000; export const MAX_PROJECT_TOOL_TIMEOUT_MS = 300_000; +/** A whole refresh, every connection included: a cold project answers `initialize` in up to 120 s, and a + * host must not hold the shared lock for the 13 minutes once measured. */ +export const DEFAULT_PROJECT_REFRESH_BUDGET_MS = 300_000; const CATALOG_FILE = 'project-tool-catalog.json'; const LOCK_STALE_MS = 10 * 60_000; const UUID = /^[\da-f]{8}(?:-[\da-f]{4}){3}-[\da-f]{12}$/i; @@ -159,16 +170,19 @@ function success(result: ActionResult): T { return result.data; } /** Every saved connection in scope and its current tools: `mcp-list` (read-only), then one `mcp-tools` - * per connection, two at a time (the shared transport's per-origin limit). A connection whose catalog - * cannot be read now keeps its `previous` tools and is named in `unavailable`; a removed connection - * loses them; connections outside the scope are carried over from `previous` unchanged. No account - * snapshot is attached: a registered tool is bound to a saved connection, and the session for that - * connection's backend is verified when the tool is called. */ + * per connection, two at a time (the shared transport's per-origin limit), all within one budget. A + * connection whose catalog cannot be read now, or is not read before the budget runs out, stays in the + * catalog marked `unavailable` with the reason, keeping its `previous` tools when it had any, so a scope + * is never written empty and healthy when its connections exist; a removed connection loses its tools; + * connections outside the scope are carried over from `previous` unchanged. No account snapshot is + * attached: a registered tool is bound to a saved connection, and the session for that connection's + * backend is verified when the tool is called. */ export async function readProjectToolCatalog(dispatch: ProjectToolDispatch, options: { - backend?: string; environmentUid?: string; previous?: ProjectToolCatalog | null; signal?: AbortSignal; + backend?: string; environmentUid?: string; previous?: ProjectToolCatalog | null; signal?: AbortSignal; budgetMs?: number; /** The session status already read for this refresh; otherwise `auth-status` is dispatched first. */ session?: SessionStatus; -} = {}): Promise<{ catalog: ProjectToolCatalog; backend: string; unavailable: string[]; retained: number }> { +} = {}): Promise<{ catalog: ProjectToolCatalog; backend: string; unavailable: string[]; retained: number; durationMs: number }> { + const started = Date.now(), budgetMs = options.budgetMs ?? DEFAULT_PROJECT_REFRESH_BUDGET_MS; const status = options.session ?? success(await dispatch({ action: 'auth-status', ...(options.backend ? { backend: options.backend } : {}) })) as SessionStatus; if (!status.authenticated || !status.backend) throw new ToolingError('NOT_AUTHENTICATED', 'Sign in to MainSequence to register saved project MCP tools.'); const backend = status.backend, environmentUid = options.environmentUid?.toLowerCase(); @@ -177,34 +191,47 @@ export async function readProjectToolCatalog(dispatch: ProjectToolDispatch, opti const rows = success(await dispatch({ action: 'mcp-list', backend, readOnly: true, ...environment })) as McpConnection[]; const previous = options.previous?.connections ?? []; const connections = previous.filter(connection => !inScope(connection, scope)), unavailable: string[] = []; - let retained = 0; + let retained = 0, expired = false, timer: NodeJS.Timeout | undefined; + const budget = new Promise<'budget'>(resolve => { timer = setTimeout(() => { expired = true; resolve('budget'); }, Math.max(0, budgetMs - (Date.now() - started))); }); + const seconds = budgetMs >= 1000 ? `${Math.round(budgetMs / 1000)} s` : `${budgetMs} ms`; + function unreadable(row: McpConnection, code: string, message: string): void { + const earlier = previous.find(connection => connection.backend === backend && connection.releaseUid === row.resource_release_uid.toLowerCase()); + if (earlier) retained += earlier.tools.length; + connections.push({ ...(earlier ?? projectToolConnection(row)), unavailable: { at: new Date().toISOString(), code, message } }); + unavailable.push(projectToolConnectionLabel(row)); + } const pending = [...rows]; - await Promise.all(Array.from({ length: Math.min(2, pending.length) }, async () => { - while (pending.length) { - options.signal?.throwIfAborted(); - const row = pending.shift()!; - try { - const output = success(await dispatch({ action: 'mcp-tools', backend, releaseUid: row.resource_release_uid, ...environment })) as McpCatalog; - const current = output.connection; - connections.push(projectToolConnection({ ...row, name: row.name ?? current?.name, display_name: row.display_name ?? current?.display_name, - mcp_connection: { ...row.mcp_connection, organization_environment_name: row.mcp_connection.organization_environment_name ?? current?.mcp_connection?.organization_environment_name } }, output.tools)); - } catch (error) { - if (error instanceof ToolingError && error.code === 'CANCELLED') throw error; + try { + await Promise.all(Array.from({ length: Math.min(2, pending.length) }, async () => { + while (pending.length && !expired) { options.signal?.throwIfAborted(); - const earlier = previous.find(connection => connection.backend === backend && connection.releaseUid === row.resource_release_uid.toLowerCase()); - if (earlier) { connections.push(earlier); retained += earlier.tools.length; } - unavailable.push(projectToolConnectionLabel(row)); + const row = pending.shift()!; + try { + const outcome = await Promise.race([Promise.resolve(dispatch({ action: 'mcp-tools', backend, releaseUid: row.resource_release_uid, ...environment })).then(result => ({ result })), budget]); + if (outcome === 'budget') { unreadable(row, 'TIMEOUT', `Not read within the ${seconds} refresh budget; its earlier tools stay registered until a later refresh reads it.`); break; } + const output = success(outcome.result) as McpCatalog, current = output.connection; + connections.push(projectToolConnection({ ...row, name: row.name ?? current?.name, display_name: row.display_name ?? current?.display_name, + mcp_connection: { ...row.mcp_connection, organization_environment_name: row.mcp_connection.organization_environment_name ?? current?.mcp_connection?.organization_environment_name } }, output.tools)); + } catch (error) { + if (error instanceof ToolingError && error.code === 'CANCELLED') throw error; + options.signal?.throwIfAborted(); + unreadable(row, error instanceof ToolingError ? error.code : 'INTERNAL_ERROR', error instanceof ToolingError ? error.message : 'The catalog could not be read.'); + } } - } - })); + })); + // Whatever the budget left unread is reported, never silently dropped. + for (const row of pending) unreadable(row, 'TIMEOUT', `Not read within the ${seconds} refresh budget; its earlier tools stay registered until a later refresh reads it.`); + } finally { clearTimeout(timer); } connections.sort((a, b) => a.backend.localeCompare(b.backend) || a.environmentUid.localeCompare(b.environmentUid) || a.releaseUid.localeCompare(b.releaseUid)); - return { catalog: { version: 1, updatedAt: new Date().toISOString(), connections }, backend, unavailable, retained }; + return { catalog: { version: 1, updatedAt: new Date().toISOString(), connections }, backend, unavailable, retained, durationMs: Date.now() - started }; } function connectionRecord(value: unknown): ProjectToolConnection | undefined { if (!object(value) || typeof value.backend !== 'string' || !UUID.test(String(value.releaseUid)) || !UUID.test(String(value.environmentUid)) || !Array.isArray(value.tools)) return; + const u = value.unavailable; return { backend: value.backend, releaseUid: String(value.releaseUid).toLowerCase(), environmentUid: String(value.environmentUid).toLowerCase(), - environmentName: text(value.environmentName), name: text(value.name), tools: normalizeTools(value.tools) }; + environmentName: text(value.environmentName), name: text(value.name), tools: normalizeTools(value.tools), + ...(object(u) && typeof u.code === 'string' && typeof u.message === 'string' ? { unavailable: { at: text(u.at, 40), code: text(u.code, 60), message: text(u.message, 1000) } } : {}) }; } /** Beside `mcp-connections.json`, shared by every host on the machine. */ export const projectToolCatalogPath = (env: NodeJS.ProcessEnv = process.env): string => path.join(mainSequenceStateDirectory(env), CATALOG_FILE); @@ -229,10 +256,12 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { const file = options.file ?? projectToolCatalogPath(options.env), staleMs = options.staleMs ?? 15 * 60_000; let catalog = readProjectToolCatalogFile(file), loadedAt = mtime(), running: Promise | null = null; let current: { controller: AbortController; locked: boolean } | null = null; - // The scope of the last refresh: the current account's backend (learned from auth-status), so a - // listing shows this host's connections even when another backend's are cached beside them. - let known: ProjectToolScope | undefined; - const state = { unavailable: [] as string[], retained: 0, error: null as ProjectToolCatalogState['error'] }; + // The backend this host's listing is for: given by the host, resolved by ready() the way auth-status + // resolves it, or learned from a refresh. The file is shared by every host and backend on the machine, + // so until it is known a listing shows nothing rather than another backend's tools. + let known: ProjectToolScope | undefined = options.backend ? { backend: options.backend } : undefined; + let readying: Promise | null = null; + const state = { error: null as ProjectToolCatalogState['error'] }; function mtime(): number { try { return fs.statSync(file).mtimeMs; } catch { return 0; } } /** Another process may have refreshed the shared cache since this one read it. */ function reload(): void { @@ -240,7 +269,9 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { if (now === loadedAt) return; catalog = readProjectToolCatalogFile(file); loadedAt = now; } - const registration = (scope?: ProjectToolScope) => projectToolRegistration(catalog, scope ?? known); + const none = { definitions: [] as ProjectToolDefinition[], bindings: new Map() }; + const registration = (scope?: ProjectToolScope) => (scope ?? known) ? projectToolRegistration(catalog, scope ?? known) : none; + const selected = (scope?: ProjectToolScope) => (catalog?.connections ?? []).filter(connection => (scope ?? known) ? inScope(connection, scope ?? known) : false); async function write(next: ProjectToolCatalog): Promise { await fsp.mkdir(path.dirname(file), { recursive: true, mode: 0o700 }); const temporary = `${file}.${randomUUID()}.tmp`; @@ -278,16 +309,26 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { } return { file, - /** Definitions for the host's tool list, from the newest shared cache. */ + /** The backend this listing is for, resolved once without any network request (the process + * environment, then the shared preferences, then the default), as `auth-status` resolves it. A + * host calls this before its first tool list; a refresh or an explicit scope also sets it. */ + ready(): Promise { + if (known) return Promise.resolve(); + readying ??= resolveBackend(undefined, undefined, options.env ?? process.env).then(backend => { known ??= { backend }; }, () => { /* no preference and no default: listings stay empty until a refresh learns the backend */ }); + return readying; + }, + /** Definitions for the host's tool list, from the newest shared cache, for the known backend only. */ list(scope?: ProjectToolScope): ProjectToolDefinition[] { reload(); return registration(scope).definitions; }, binding(name: string): ProjectToolBinding | undefined { reload(); return registration({}).bindings.get(name); }, state(scope?: ProjectToolScope): ProjectToolCatalogState { reload(); - const selected = scope ?? known; + const connections = selected(scope), unavailable = connections.filter(connection => connection.unavailable); return { count: registration(scope).definitions.length, - connections: (catalog?.connections ?? []).filter(connection => inScope(connection, selected)).map(connection => ({ backend: connection.backend, releaseUid: connection.releaseUid, - environmentUid: connection.environmentUid, environmentName: connection.environmentName, name: connection.name, tools: connection.tools.length })), - updatedAt: catalog?.updatedAt || null, refreshing: running !== null, unavailable: state.unavailable, retained: state.retained, error: state.error }; + connections: connections.map(connection => ({ backend: connection.backend, releaseUid: connection.releaseUid, environmentUid: connection.environmentUid, environmentName: connection.environmentName, + name: connection.name, tools: connection.tools.length, ...(connection.unavailable ? { unavailable: connection.unavailable } : {}) })), + updatedAt: catalog?.updatedAt || null, refreshing: running !== null, + // Read from the file, so a host whose process did not run the refresh still sees what it could not read. + unavailable: unavailable.map(connection => connection.name || connection.releaseUid), retained: unavailable.reduce((sum, connection) => sum + connection.tools.length, 0), error: state.error }; }, /** The state plus definitions and bindings, as the `mcp-catalog` action returns them. */ view(scope?: ProjectToolScope): ProjectToolCatalogState & { definitions: ProjectToolDefinition[]; bindings: Record } { @@ -321,9 +362,9 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { current!.locked = true; try { reload(); - const read = await readProjectToolCatalog(dispatch, { ...scope, backend: session.backend, previous: catalog, signal: controller.signal, session }); + const read = await readProjectToolCatalog(dispatch, { ...scope, backend: session.backend, previous: catalog, signal: controller.signal, session, budgetMs: options.budgetMs }); await write(read.catalog); - state.unavailable = read.unavailable; state.retained = read.retained; state.error = null; + state.error = null; adopt(read.catalog, scope ?? { backend: read.backend }); return true; } finally { current!.locked = false; await release(); } @@ -331,7 +372,10 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { state.error = { code: error instanceof ToolingError ? error.code : 'INTERNAL_ERROR', message: error instanceof ToolingError ? error.message : 'The saved project MCP tools could not be refreshed.' }; return false; } - })().finally(() => { running = null; current = null; }); + })().finally(() => { + // A read the budget left behind must not keep a socket or the dispatcher alive. + controller.abort(); running = null; current = null; + }); return running; }, /** A host process is ending: stop its refresh and free the shared lock for the next one at once. */ @@ -346,7 +390,8 @@ export function createProjectToolCatalog(options: ProjectToolCatalogOptions) { reload(); const index = catalog?.connections.findIndex(item => item.releaseUid === releaseUid.toLowerCase() && (!backend || item.backend === backend)) ?? -1; if (!catalog || index < 0) { void this.refresh(true); return; } - const connections = catalog.connections.map((item, i) => i === index ? { ...item, tools: normalizeTools(tools) } : item); + // A catalog just read is a successful read: the connection is available again. + const connections = catalog.connections.map((item, i) => i === index ? { ...item, tools: normalizeTools(tools), unavailable: undefined } : item); const next: ProjectToolCatalog = { version: 1, updatedAt: new Date().toISOString(), connections }; try { await write(next); adopt(next); } catch { /* the in-memory tools are already current */ } }, diff --git a/tests/project-tools.test.cjs b/tests/project-tools.test.cjs index 27c9808..027dfed 100644 --- a/tests/project-tools.test.cjs +++ b/tests/project-tools.test.cjs @@ -195,21 +195,75 @@ test('without a session nothing is read and the state says so; a signed-out refr assert.equal(f.actions.length, count + 1); assert.equal(f.catalog.state().error.code, 'NOT_AUTHENTICATED'); assert.equal(f.catalog.list().length, 1); }); -test('a listing is scoped to the current backend once a refresh learned it, so another backend\'s cached connections stay out of a host\'s tool list', async t => { +test('a listing is empty until the host\'s backend is known, then scoped to it: another backend\'s cached connections never enter a host\'s tool list', async t => { const elsewhere = { backend: 'https://elsewhere.example.test', releaseUid, environmentUid, environmentName: 'Research', name: 'Elsewhere', tools: [metrics] }; const f = await fixture(t, healthy()); await fs.mkdir(path.dirname(f.file), { recursive: true }); await fs.writeFile(f.file, JSON.stringify({ version: 1, updatedAt: new Date().toISOString(), connections: [elsewhere] })); - const catalog = createProjectToolCatalog({ file: f.file, dispatch: () => healthy(), notify: () => { f.changed = (f.changed || 0) + 1; } }); - assert.equal(catalog.list().length, 1, 'before the backend is known the whole cache is listed'); - assert.equal(await catalog.refresh(), false, 'the cache is fresh'); - assert.deepEqual(catalog.list(), [], 'knowing the backend scopes the listing'); assert.equal(f.changed, 1, 'and the change is announced'); - assert.equal(catalog.list({ backend: elsewhere.backend }).length, 1); assert.ok(catalog.binding(projectToolName(elsewhere.backend, environmentUid, releaseUid, 'metrics')), 'a binding resolves whatever its backend'); - await catalog.refresh(true); - assert.deepEqual(catalog.list().map(d => d.description), ['MainSequence Metrics [Research]: Inspect metrics']); + // The first release listed every cached scope on a fresh server's first tools/list. + const unknown = createProjectToolCatalog({ file: f.file, dispatch: () => healthy() }); + assert.deepEqual(unknown.list(), [], 'before the backend is known nothing is listed'); + assert.equal(unknown.state().count, 0); assert.deepEqual(unknown.state().connections, []); assert.deepEqual(unknown.view().definitions, []); + // ready() resolves the backend without a network request, the way auth-status does: here from the environment. + const resolved = createProjectToolCatalog({ file: f.file, env: { MAINSEQUENCE_ENDPOINT: elsewhere.backend }, dispatch: () => { throw new Error('ready() must not dispatch'); } }); + await resolved.ready(); await resolved.ready(); + assert.equal(resolved.list().length, 1, 'the resolved backend\'s connection is listed'); + const given = createProjectToolCatalog({ file: f.file, backend, dispatch: () => healthy(), notify: () => { f.changed = (f.changed || 0) + 1; } }); + assert.deepEqual(given.list(), [], 'a given backend scopes the listing at once'); + assert.equal(await given.refresh(), false, 'the cache is fresh'); + assert.equal(given.list({ backend: elsewhere.backend }).length, 1); assert.ok(given.binding(projectToolName(elsewhere.backend, environmentUid, releaseUid, 'metrics')), 'a binding resolves whatever its backend'); + await given.refresh(true); + assert.deepEqual(given.list().map(d => d.description), ['MainSequence Metrics [Research]: Inspect metrics']); assert.equal(f.changed, 1, 'the change is announced once'); assert.equal(readProjectToolCatalogFile(f.file).connections.length, 2, 'the other backend\'s connection is kept in the shared file'); }); +test('a connection whose read fails is written as unavailable with its earlier tools, never as an empty healthy scope; the mark is in the file for every host and clears on the next successful read', async t => { + let mode = 'failing'; + const f = await fixture(t, request => mode === 'failing' && request.action === 'mcp-tools' + ? failure(request.action, 'BACKEND_UNREACHABLE', 'The project MCP initialize request to https://project.example.test timed out after 120 s. It was not retried.') : healthy()(request)); + assert.equal(await f.catalog.refresh(true), true, 'the refresh completes and writes what it learned'); + const written = readProjectToolCatalogFile(f.file); + assert.equal(written.connections.length, 1, 'the connection exists in the saved connections, so it exists in the catalog'); + assert.deepEqual(written.connections[0].tools, []); assert.equal(written.connections[0].unavailable.code, 'BACKEND_UNREACHABLE'); + assert.match(written.connections[0].unavailable.message, /timed out after 120 s/); assert.ok(Date.parse(written.connections[0].unavailable.at) > 0); + assert.deepEqual(f.catalog.list(), [], 'no tool is registered for it'); + const state = f.catalog.state(); + assert.deepEqual(state.unavailable, ['Metrics']); assert.equal(state.retained, 0); assert.equal(state.error, null); assert.equal(state.connections[0].unavailable.code, 'BACKEND_UNREACHABLE'); + // Another host's process (the mcp-catalog action is one) reads the mark from the file. + const other = createProjectToolCatalog({ file: f.file, backend, dispatch: () => healthy() }); + assert.deepEqual(other.state().unavailable, ['Metrics']); assert.deepEqual(other.view({ backend }).unavailable, ['Metrics']); + mode = 'healthy'; await f.catalog.refresh(true); + assert.equal(readProjectToolCatalogFile(f.file).connections[0].unavailable, undefined, 'a successful read clears the mark'); + assert.deepEqual(f.catalog.state().unavailable, []); assert.equal(f.catalog.list().length, 1); + mode = 'failing'; await f.catalog.refresh(true); + assert.equal(f.catalog.list().length, 1, 'a later failure keeps the tools it had'); assert.deepEqual(f.catalog.state().unavailable, ['Metrics']); assert.equal(f.catalog.state().retained, 1); + assert.equal(readProjectToolCatalogFile(f.file).connections[0].unavailable.code, 'BACKEND_UNREACHABLE'); + mode = 'healthy'; await f.catalog.update(releaseUid, [metrics]); + assert.equal(readProjectToolCatalogFile(f.file).connections[0].unavailable, undefined, 'an explicit catalog read is a successful read'); +}); + +test('a refresh ends within its budget: connections not read by then are unavailable, their earlier tools stay, and the dispatcher is released', async t => { + const second = { ...row, resource_release_uid: '30000000-0000-4000-8000-000000000002', name: 'Slow' }; + const root = await fs.mkdtemp(path.join(os.tmpdir(), 'ms-catalog-')); t.after(() => fs.rm(root, { recursive: true, force: true })); + const file = path.join(root, 'catalog.json'), signals = []; + await fs.writeFile(file, JSON.stringify({ version: 1, updatedAt: '2020-01-01T00:00:00.000Z', connections: [{ backend, releaseUid, environmentUid, environmentName: 'Research', name: 'Metrics', tools: [metrics] }] })); + const hanging = request => request.action === 'mcp-tools' ? new Promise(() => {}) : healthy(undefined, [row, second])(request); + const catalog = createProjectToolCatalog({ file, budgetMs: 200, dispatch: signal => { signals.push(signal); return hanging; } }); + const started = Date.now(); + assert.equal(await catalog.refresh(true), true); + assert.ok(Date.now() - started < 2_000, 'the refresh returned at its budget, not after every hung read'); + const state = catalog.state(); + assert.deepEqual(state.unavailable, ['Metrics', 'Slow']); assert.equal(state.retained, 1); assert.equal(state.error, null); + assert.equal(catalog.list().length, 1, 'Metrics keeps its earlier tool'); + const written = readProjectToolCatalogFile(file); + assert.ok(written.connections.every(connection => connection.unavailable.code === 'TIMEOUT' && /within the 200 ms refresh budget/.test(connection.unavailable.message)), JSON.stringify(written.connections)); + assert.ok(signals.length && signals.every(signal => signal.aborted), 'the hung reads were abandoned with the refresh'); + // The reader alone reports how long it took and what it left unread. + const read = await readProjectToolCatalog(hanging, { backend, budgetMs: 100 }); + assert.ok(read.durationMs >= 100 && read.durationMs < 1_500); assert.deepEqual(read.unavailable, ['Metrics', 'Slow']); assert.equal(read.catalog.connections.length, 2); + assert.ok(readProjectToolCatalog(healthy(), { backend }).then(result => result.durationMs >= 0 && result.unavailable.length === 0)); +}); + test('one process refreshes at a time; the others reuse its file, a stale lock is cleared, and a waiting refresh takes the lock when it is freed', async t => { const f = await fixture(t, healthy()); await fs.mkdir(path.dirname(f.file), { recursive: true }); @@ -219,7 +273,7 @@ test('one process refreshes at a time; the others reuse its file, a stale lock i assert.equal(f.catalog.state().error, null, 'a background refresh that found a live lock is not an error'); const old = new Date(Date.now() - 11 * 60_000); await fs.utimes(f.file + '.lock', old, old); await f.catalog.refresh(true); assert.equal(f.actions.length, 4); assert.equal(await fs.stat(f.file + '.lock').catch(e => e.code), 'ENOENT'); - const other = createProjectToolCatalog({ file: f.file, dispatch: () => async request => { if (request.action === 'auth-status') return healthy()(request); throw new Error('the other process must not read'); } }); + const other = createProjectToolCatalog({ file: f.file, backend, dispatch: () => async request => { if (request.action === 'auth-status') return healthy()(request); throw new Error('the other process must not read'); } }); assert.equal(other.list().length, 1, 'a second server process reads the shared cache'); await new Promise(resolve => setTimeout(resolve, 20)); await f.catalog.update(releaseUid, [metrics, { name: 'export', inputSchema: { type: 'object' } }]); @@ -253,7 +307,7 @@ test('a disabled catalog still serves the cache but never reads or writes on its const root = await fs.mkdtemp(path.join(os.tmpdir(), 'ms-catalog-')); t.after(() => fs.rm(root, { recursive: true, force: true })); const file = path.join(root, 'catalog.json'); await fs.writeFile(file, JSON.stringify({ version: 1, updatedAt: '2020-01-01T00:00:00.000Z', connections: [{ backend, releaseUid, environmentUid, environmentName: 'Research', name: 'Metrics', tools: [metrics] }] })); - const catalog = createProjectToolCatalog({ file, disabled: true, dispatch: () => async () => { throw new Error('disabled catalogs do not dispatch'); } }); + const catalog = createProjectToolCatalog({ file, backend, disabled: true, dispatch: () => async () => { throw new Error('disabled catalogs do not dispatch'); } }); assert.equal(catalog.list().length, 1); assert.equal(await catalog.refresh(true), false); await catalog.update(releaseUid, []); assert.equal(catalog.list().length, 1); @@ -332,6 +386,10 @@ test('mcp-catalog answers from the shared file at once, re-reads the saved conne const degraded = await f.create().dispatch({ action: 'mcp-catalog', backend, refresh: 'force' }); assert.equal(degraded.status, 'success', degraded.summary); assert.deepEqual(degraded.data.unavailable, ['metatable-mcp']); assert.equal(degraded.data.retained, 1); assert.equal(degraded.data.count, 1); assert.match(degraded.summary, /metatable-mcp could not be read now; earlier tools stay registered/); + assert.equal(degraded.data.connections[0].unavailable.code, 'BACKEND_UNREACHABLE'); + // The next process (a Claude Desktop worker is one per action) sees the mark from the file, with no refresh of its own. + const seen = await f.create().dispatch({ action: 'mcp-catalog', backend }); + assert.equal(seen.data.refreshed, false); assert.deepEqual(seen.data.unavailable, ['metatable-mcp']); assert.match(seen.summary, /could not be read now/); }); test('mcp-tool-call answers in the shared result shape and honours a tool\'s advertised timeout', async t => {