diff --git a/.env.example b/.env.example index 87de1ff2..d1356ad7 100644 --- a/.env.example +++ b/.env.example @@ -253,6 +253,7 @@ DB_PASSWORD=pedscribe_secret_change_me # CLINICAL_ASSISTANT_MCP_INITIALIZE_TIMEOUT_MS=30000 # CLINICAL_ASSISTANT_MCP_REQUEST_TIMEOUT_MS=90000 # CLINICAL_ASSISTANT_MCP_SESSION_TTL_MS=600000 +# CLINICAL_ASSISTANT_MCP_CONCURRENCY=3 # library searches in flight at once; they used to run one at a time # CLINICAL_ASSISTANT_MCP_WARMUP= # open a session at boot # CLINICAL_ASSISTANT_MCP_WARMUP_DELAY_MS= diff --git a/docs/my-resources.md b/docs/my-resources.md index d018dd1b..6d05cff6 100644 --- a/docs/my-resources.md +++ b/docs/my-resources.md @@ -421,3 +421,21 @@ generating — it had none, so "add what the 2024 trial showed" was answered fro the model's memory rather than by looking anything up. The previous version is replaced, not versioned. + +## Sharing + +A resource is its author's. `POST /api/my-resources/:id/shares` (by email, +exact — accounts are never listed) and `PUT …/shares/all` extend *reading* — +open, preview, download — to named people or to everyone signed in; the rows +live in `user_resource_shares` and `user_resources.shared_with_all`. Every +write route (modify, theme, delete, the share list itself) still filters on +`user_id`. The list route returns a person's own resources first, then what is +shared with them, each row saying `owned` and `shared_by_name`. + +## Preview + +`GET /api/my-resources/:id/preview` renders the resource the way its download +is built and turns it into one PNG per page (`src/utils/previewPages.js`: +Gotenberg to PDF, `pdftoppm` to pages), keyed on `updated_at` and theme and +kept under the OS temp directory; `…/preview/:page` serves a page. Theme +samples have the same pair under `theme-sample/:id/preview`. diff --git a/migrations/1781300000000_resource-shares.js b/migrations/1781300000000_resource-shares.js new file mode 100644 index 00000000..4009a202 --- /dev/null +++ b/migrations/1781300000000_resource-shares.js @@ -0,0 +1,27 @@ +// Sharing a resource with other people on this site. +// +// A resource was private to its author with no way out but the author's own +// Nextcloud. A share is a row per (resource, person): the person can open, +// preview and download it — not modify, re-skin or delete it — and the author +// can withdraw it. shared_with_all opens a resource to every signed-in account +// without naming them. +// +// Rows go with the resource and with the person: a deleted account leaves no +// dangling grant, and a deleted resource leaves no orphan share. + +exports.up = pgm => pgm.sql(` + ALTER TABLE user_resources ADD COLUMN IF NOT EXISTS shared_with_all BOOLEAN NOT NULL DEFAULT FALSE; + CREATE TABLE IF NOT EXISTS user_resource_shares ( + resource_id INTEGER NOT NULL REFERENCES user_resources(id) ON DELETE CASCADE, + user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + shared_by INTEGER REFERENCES users(id) ON DELETE SET NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (resource_id, user_id) + ); + CREATE INDEX IF NOT EXISTS idx_resource_shares_user ON user_resource_shares (user_id); +`); + +exports.down = pgm => pgm.sql(` + DROP TABLE IF EXISTS user_resource_shares; + ALTER TABLE user_resources DROP COLUMN IF EXISTS shared_with_all; +`); diff --git a/public/components/faq.html b/public/components/faq.html index 721cefe9..0fdc1749 100644 --- a/public/components/faq.html +++ b/public/components/faq.html @@ -278,7 +278,8 @@
-

Download as PowerPoint, Word or PDF, or send the rendered file straight to your Nextcloud. Articles download as Word or PDF. Sharing a resource with another person is not available yet.

+

Preview shows every page in place, on a phone too. Download as PowerPoint, Word or PDF, or send the rendered file straight to your Nextcloud; articles download as Word or PDF.

+

Share lets other people on this site open, preview and download it — one person at a time by email, or everyone signed in with one switch — without being able to change it. What others share with you appears in your library marked "Shared by …".

diff --git a/public/js/myResources.js b/public/js/myResources.js index 60a954c0..9d893f89 100644 --- a/public/js/myResources.js +++ b/public/js/myResources.js @@ -677,7 +677,8 @@ return; } select.disabled = false; - library.forEach(function (row) { + // Only what is mine can be modified; a share is read-only. + library.filter(function (row) { return row.owned !== false; }).forEach(function (row) { var option = document.createElement('option'); option.value = String(row.id); option.textContent = (row.title || 'Untitled') + @@ -785,6 +786,17 @@ // Only worth saying when it is the weaker kind. A deck is the normal case. (row.kind !== 'article' && row.has_deck === false ? ' · plain text, no slide layout' : ''); + if (row.owned === false && row.shared_by_name) { + var from = document.createElement('div'); + from.style.cssText = 'font-size:11px;color:var(--blue);'; + from.textContent = 'Shared by ' + row.shared_by_name; + body.appendChild(from); + } else if (row.owned !== false && row.shared_with_all) { + var all = document.createElement('div'); + all.style.cssText = 'font-size:11px;color:var(--g500);'; + all.textContent = 'Shared with everyone'; + body.appendChild(all); + } body.appendChild(title); body.appendChild(meta); wrap.appendChild(body); @@ -817,6 +829,17 @@ // Re-skinning is a column write, not a regeneration: the next download // renders from the same deck in different colours. Only for a row that has // a deck — flat markdown has no palette to change. + // Reading is what a share gives; changing is the author's. + if (row.owned === false) { return wrap; } + + var share = document.createElement('button'); + share.className = 'btn-sm btn-ghost'; + share.type = 'button'; + share.dataset.share = String(row.id); + share.innerHTML = ' Share'; + share.title = 'Share with people on this site'; + wrap.appendChild(share); + // Every presentation takes a theme now — markdown slides too. if (row.kind !== 'article' && themeCatalogue.length > 1) { var theme = document.createElement('select'); @@ -918,6 +941,8 @@ var cloud = event.target.closest && event.target.closest('[data-nextcloud]'); if (cloud) return sendToNextcloud(cloud.dataset.nextcloud, cloud.dataset.format, cloud); + var shareBtn = event.target.closest && event.target.closest('[data-share]'); + if (shareBtn) { openSharePanel(shareBtn.dataset.share, shareBtn.closest('.saved-enc-item')); return; } var preview = event.target.closest && event.target.closest('[data-preview]'); if (preview) { openPreview('/api/my-resources/' + encodeURIComponent(preview.dataset.preview) + '/preview', preview.dataset.previewTitle || 'Preview'); return; } var download = event.target.closest && event.target.closest('[data-download]'); @@ -960,6 +985,68 @@ .finally(function () { btn.disabled = false; btn.textContent = original; }); } + // ── Sharing ────────────────────────────────────────────── + // Under the row: a switch for everyone, an email to add one person, and the + // list of people it is shared with, each with a way to withdraw. + function openSharePanel(id, rowEl) { + if (!rowEl) return; + var existing = rowEl.querySelector('.mr-share'); + if (existing) { existing.remove(); return; } + var panel = document.createElement('div'); + panel.className = 'mr-share'; + panel.style.cssText = 'flex-basis:100%;margin-top:8px;padding:10px 12px;border:1px solid var(--g200);border-radius:8px;background:var(--g50);display:grid;gap:8px;font-size:13px;'; + panel.innerHTML = '' + + '
' + + '
' + + '
'; + rowEl.appendChild(panel); + var base = '/api/my-resources/' + encodeURIComponent(id) + '/shares'; + function paint(state) { + panel.querySelector('.mr-share-all').checked = !!state.sharedWithAll; + var list = panel.querySelector('.mr-share-people'); + list.innerHTML = ''; + if (!state.people.length) { + var none = document.createElement('div'); none.style.cssText = 'font-size:12px;color:var(--g500);'; + none.textContent = state.sharedWithAll ? 'Shared with everyone.' : 'Not shared with anyone yet.'; + list.appendChild(none); return; + } + state.people.forEach(function (p) { + var line = document.createElement('div'); + line.style.cssText = 'display:flex;align-items:center;gap:8px;'; + var who = document.createElement('span'); who.style.flex = '1'; + who.textContent = (p.name ? p.name + ' · ' : '') + p.email; + var out = document.createElement('button'); out.type = 'button'; out.className = 'btn-sm btn-ghost'; + out.textContent = 'Remove'; out.title = 'Withdraw the share'; + out.addEventListener('click', function () { + fetch(base + '/' + encodeURIComponent(p.id), { method: 'DELETE', headers: getAuthHeaders() }) + .then(function (r) { return r.json(); }).then(load).catch(function (e) { showToast(e.message, 'error'); }); + }); + line.appendChild(who); line.appendChild(out); list.appendChild(line); + }); + } + function load() { + return fetch(base, { headers: getAuthHeaders() }).then(function (r) { return r.json(); }) + .then(function (d) { if (!d.success) throw new Error(d.error || 'Could not load'); paint(d); loadLibrary(); }) + .catch(function (e) { showToast(e.message, 'error'); }); + } + panel.querySelector('.mr-share-all').addEventListener('change', function (e) { + fetch(base + '/all', { method: 'PUT', headers: getAuthHeaders(), body: JSON.stringify({ value: e.target.checked }) }) + .then(function (r) { return r.json(); }) + .then(function (d) { if (!d.success) throw new Error(d.error || 'Could not change sharing'); showToast(d.sharedWithAll ? 'Shared with everyone on the site' : 'No longer shared with everyone', 'success'); return load(); }) + .catch(function (err) { showToast(err.message, 'error'); load(); }); + }); + panel.querySelector('.mr-share-add').addEventListener('click', function () { + var input = panel.querySelector('.mr-share-email'); + var email = (input.value || '').trim(); + if (!email) { input.focus(); return; } + fetch(base, { method: 'POST', headers: getAuthHeaders(), body: JSON.stringify({ email: email }) }) + .then(function (r) { return r.json(); }) + .then(function (d) { if (!d.success) throw new Error(d.error || 'Could not share'); input.value = ''; showToast('Shared with ' + (d.person.name || d.person.email), 'success'); return load(); }) + .catch(function (err) { showToast(err.message, 'error'); }); + }); + load(); + } + // ── The preview gallery ────────────────────────────────── // One overlay: the pages of a document, top to bottom, at the width of the // screen. Each page is fetched with the auth header (an cannot diff --git a/src/routes/myResources.js b/src/routes/myResources.js index 58e6d964..f5d5d11f 100644 --- a/src/routes/myResources.js +++ b/src/routes/myResources.js @@ -587,6 +587,77 @@ router.post('/my-resources/:id/to-nextcloud', async function (req, res) { } }); +// ── Sharing ───────────────────────────────────────────────── +// A resource is the author's. Reading it — open, preview, download — is +// extended to people the author named, or to everyone signed in when the +// author said so. Writing it never is: modify, re-skin, delete and the share +// list itself stay with the author, and every route that writes still filters +// on user_id as before. +async function readableResource(id, userId, columns) { + return db.get( + 'SELECT ' + columns.split(',').map(function (c) { return 'r.' + c.trim(); }).join(', ') + + ' FROM user_resources r LEFT JOIN user_resource_shares s ON s.resource_id = r.id AND s.user_id = ? ' + + 'WHERE r.id = ? AND (r.user_id = ? OR r.shared_with_all OR s.user_id IS NOT NULL)', + [userId, parseInt(id, 10), userId] + ); +} + +async function ownedResource(id, userId) { + return db.get('SELECT id, title, shared_with_all FROM user_resources WHERE id = ? AND user_id = ?', [parseInt(id, 10), userId]); +} + +router.get('/my-resources/:id/shares', async function (req, res) { + try { + var row = await ownedResource(req.params.id, req.user.id); + if (!row) return res.status(404).json({ error: 'Not found' }); + var people = await db.all( + 'SELECT u.id, u.name, u.email FROM user_resource_shares s JOIN users u ON u.id = s.user_id ' + + 'WHERE s.resource_id = ? ORDER BY s.created_at', [row.id]); + res.json({ success: true, sharedWithAll: !!row.shared_with_all, people: people }); + } catch (err) { res.status(500).json({ error: 'Could not load the share list' }); } +}); + +// Everyone signed in, or not. A switch rather than a list, since "the whole +// department" has no useful list. +router.put('/my-resources/:id/shares/all', async function (req, res) { + try { + var row = await ownedResource(req.params.id, req.user.id); + if (!row) return res.status(404).json({ error: 'Not found' }); + var on = req.body.value === true || String(req.body.value) === 'true'; + await db.run('UPDATE user_resources SET shared_with_all = ? WHERE id = ? AND user_id = ?', [on, row.id, req.user.id]); + logger.audit(req.user.id, on ? 'resource_share_all' : 'resource_unshare_all', 'Resource ' + row.id + (on ? ' shared with everyone' : ' no longer shared with everyone'), req, { category: 'clinical' }); + res.json({ success: true, sharedWithAll: on }); + } catch (err) { res.status(500).json({ error: 'Could not change sharing' }); } +}); + +// By email, exactly. Nothing here lists accounts: an address that is not one +// is refused, and the author is told so, without turning this into a +// directory. +router.post('/my-resources/:id/shares', async function (req, res) { + try { + var row = await ownedResource(req.params.id, req.user.id); + if (!row) return res.status(404).json({ error: 'Not found' }); + var email = String(req.body.email || '').trim().toLowerCase(); + if (!email || email.length > 320) return res.status(400).json({ error: 'Enter the person\'s email' }); + var person = await db.get('SELECT id, name, email FROM users WHERE email = ? AND disabled IS NOT TRUE', [email]); + if (!person) return res.status(404).json({ error: 'No account with that email on this site' }); + if (person.id === req.user.id) return res.status(400).json({ error: 'That is you' }); + await db.run('INSERT INTO user_resource_shares (resource_id, user_id, shared_by) VALUES (?, ?, ?) ON CONFLICT DO NOTHING', + [row.id, person.id, req.user.id]); + logger.audit(req.user.id, 'resource_share', 'Resource ' + row.id + ' shared with user ' + person.id, req, { category: 'clinical' }); + res.json({ success: true, person: person }); + } catch (err) { res.status(500).json({ error: 'Could not share it' }); } +}); + +router.delete('/my-resources/:id/shares/:userId', async function (req, res) { + try { + var row = await ownedResource(req.params.id, req.user.id); + if (!row) return res.status(404).json({ error: 'Not found' }); + await db.run('DELETE FROM user_resource_shares WHERE resource_id = ? AND user_id = ?', [row.id, parseInt(req.params.userId, 10)]); + res.json({ success: true }); + } catch (err) { res.status(500).json({ error: 'Could not withdraw the share' }); } +}); + // ── Previews: pages as pictures ───────────────────────────── // Looking without downloading, on a phone as much as anywhere. A resource is // rendered the way its download would be (PowerPoint for a presentation, @@ -612,10 +683,7 @@ async function resourceOfficeBytes(row, user) { router.get('/my-resources/:id/preview', async function (req, res) { try { - var row = await db.get( - 'SELECT id, kind, markdown, image_ids, deck, theme, updated_at FROM user_resources WHERE id = ? AND user_id = ?', - [parseInt(req.params.id, 10), req.user.id] - ); + var row = await readableResource(req.params.id, req.user.id, 'id, kind, markdown, image_ids, deck, theme, updated_at, user_id'); if (!row) return res.status(404).json({ error: 'Not found' }); var key = previewKeyFor(row); var pages = await previewPages.ensure(key, function () { return resourceOfficeBytes(row, req.user); }); @@ -628,10 +696,7 @@ router.get('/my-resources/:id/preview', async function (req, res) { router.get('/my-resources/:id/preview/:page', async function (req, res) { try { - var row = await db.get( - 'SELECT id, kind, theme, updated_at FROM user_resources WHERE id = ? AND user_id = ?', - [parseInt(req.params.id, 10), req.user.id] - ); + var row = await readableResource(req.params.id, req.user.id, 'id, kind, theme, updated_at'); if (!row) return res.status(404).json({ error: 'Not found' }); var png = await previewPages.page(previewKeyFor(row), parseInt(req.params.page, 10)); if (!png) return res.status(404).json({ error: 'No such page' }); @@ -722,11 +787,18 @@ router.get('/my-resources', async function (req, res) { // presentation without one behaves differently enough — flat layout, a // weaker modification path — that the owner should be able to see which // kind they have. - 'SELECT id, title, kind, topic, grounded_count, created_at, updated_at, ' + - "(deck IS NOT NULL AND jsonb_array_length(COALESCE(deck->'slides', '[]'::jsonb)) > 0) AS has_deck, " + - "COALESCE(deck->>'theme', '') AS theme " + - 'FROM user_resources WHERE user_id = ? ORDER BY created_at DESC LIMIT ?', - [req.user.id, MAX_PER_USER] + // Mine, then what others shared with me — with all, or with me by name — + // each row saying which, since the page offers different things for each. + 'SELECT r.id, r.title, r.kind, r.topic, r.grounded_count, r.created_at, r.updated_at, ' + + "(r.deck IS NOT NULL AND jsonb_array_length(COALESCE(r.deck->'slides', '[]'::jsonb)) > 0) AS has_deck, " + + "COALESCE(r.theme, r.deck->>'theme', '') AS theme, " + + '(r.user_id = ?) AS owned, r.shared_with_all, ' + + 'CASE WHEN r.user_id = ? THEN NULL ELSE u.name END AS shared_by_name ' + + 'FROM user_resources r JOIN users u ON u.id = r.user_id ' + + 'LEFT JOIN user_resource_shares s ON s.resource_id = r.id AND s.user_id = ? ' + + 'WHERE r.user_id = ? OR r.shared_with_all OR s.user_id IS NOT NULL ' + + 'ORDER BY (r.user_id = ?) DESC, r.created_at DESC LIMIT ?', + [req.user.id, req.user.id, req.user.id, req.user.id, req.user.id, MAX_PER_USER * 2] ); res.json({ success: true, resources: rows }); } catch (err) { @@ -737,11 +809,8 @@ router.get('/my-resources', async function (req, res) { router.get('/my-resources/:id', async function (req, res) { try { - var row = await db.get( - 'SELECT id, title, kind, topic, markdown, grounded_count, created_at, updated_at ' + - 'FROM user_resources WHERE id = ? AND user_id = ?', - [parseInt(req.params.id, 10), req.user.id] - ); + var row = await readableResource(req.params.id, req.user.id, + 'id, title, kind, topic, markdown, grounded_count, created_at, updated_at'); if (!row) return res.status(404).json({ error: 'Not found' }); res.json({ success: true, resource: row }); } catch (err) { @@ -1142,10 +1211,7 @@ router.get('/my-resources/:id/export', async function (req, res) { var format = String(req.query.format || 'pptx'); if (!documentExport.isSupported(format)) return res.status(400).json({ error: 'Unsupported format' }); - var row = await db.get( - 'SELECT title, kind, markdown, image_ids, deck, theme FROM user_resources WHERE id = ? AND user_id = ?', - [parseInt(req.params.id, 10), req.user.id] - ); + var row = await readableResource(req.params.id, req.user.id, 'title, kind, markdown, image_ids, deck, theme, user_id'); if (!row) return res.status(404).json({ error: 'Not found' }); // The UI does not offer it, but the route is the boundary that matters: diff --git a/src/utils/clinicalMcpClient.js b/src/utils/clinicalMcpClient.js index c5468030..565610af 100644 --- a/src/utils/clinicalMcpClient.js +++ b/src/utils/clinicalMcpClient.js @@ -16,6 +16,17 @@ var MCP_SESSION_TTL_MS = positiveInt(process.env.CLINICAL_ASSISTANT_MCP_SESSION_ var _mcpSession = null; var _mcpSessionPromise = null; var _mcpCallQueue = Promise.resolve(); +// How many tool calls may be in flight at once. They used to run one at a +// time behind a promise chain, so two people asking at the same moment +// waited for each other; the server handles concurrent requests on one +// session, so a small bound is enough to stop the queueing without letting a +// burst pile onto it. +var MCP_CONCURRENCY = positiveInt(process.env.CLINICAL_ASSISTANT_MCP_CONCURRENCY, 3); +var _inFlight = 0; +var _waiting = []; +// The session is renewed before it expires, in the background, so no one +// pays for a cold initialize at the start of their question. +var _warmTimer = null; var _mcpRequestId = 1; var MCP_CLEANUP_TIMEOUT_MS = 5000; var _closing = false; @@ -92,16 +103,46 @@ async function indexedTopicSuggestions(limit) { } function warmMcpSession() { + if (!_warmTimer && MCP_SESSION_TTL_MS > 0) { + _warmTimer = setInterval(function() { + if (_closing) return; + var soon = _mcpSession && _mcpSession.expiresAt - Date.now() < MCP_SESSION_TTL_MS / 3; + if (!_mcpSession || soon) { + if (soon) { var old = _mcpSession; _mcpSession = null; endMcpSession(old); } + getMcpSession().catch(function() {}); + } + }, Math.max(30000, Math.floor(MCP_SESSION_TTL_MS / 3))); + if (_warmTimer.unref) _warmTimer.unref(); + } return getMcpSession(); } +function acquireSlot() { + if (_inFlight < MCP_CONCURRENCY) { _inFlight++; return Promise.resolve(); } + return new Promise(function(resolve) { _waiting.push(resolve); }); +} + +function releaseSlot() { + var next = _waiting.shift(); + if (next) next(); else _inFlight--; +} + async function callMcpTool(name, args) { if (_closing) throw new Error('MCP client is closing'); - var queued = _mcpCallQueue.then(function() { - return callMcpToolUnlocked(name, args); - }); - _mcpCallQueue = queued.catch(function() {}); - return queued; + await acquireSlot(); + var started = Date.now(); + var sessionMs = 0; + try { + var t = Date.now(); + await getMcpSession(); + sessionMs = Date.now() - t; + return await callMcpToolUnlocked(name, args); + } finally { + releaseSlot(); + // Where a slow search spent its time: opening a session or running the + // call. The one question worth answering when "search=6657" shows up. + console.info('[clinical-assistant] mcp ' + name + ': session=' + sessionMs + 'ms total=' + (Date.now() - started) + 'ms in_flight=' + _inFlight); + } } async function callMcpToolUnlocked(name, args) { diff --git a/src/utils/openapiRoutes.js b/src/utils/openapiRoutes.js index 99533c03..bc150933 100644 --- a/src/utils/openapiRoutes.js +++ b/src/utils/openapiRoutes.js @@ -20,11 +20,16 @@ var parameters = { id: 'Identifier of the record, scoped to the signed-in account.', workflow: 'Which feature the job belongs to (for example my_resources).', key: 'Setting key, for example clinical_assistant.chat_model.', - slug: 'URL-safe name of the document.' + slug: 'URL-safe name of the document.', + userId: 'Id of the account the share was for.' }; var operations = { // ── Speech ────────────────────────────────────────────────────────── + 'GET /api/my-resources/:id/shares': { summary: 'Who a resource is shared with (owner only)' }, + 'PUT /api/my-resources/:id/shares/all': { summary: 'Share a resource with everyone signed in, or stop', requestBody: { required: true, content: { 'application/json': { schema: { type: 'object', properties: { value: { type: 'boolean' } } } } } } }, + 'POST /api/my-resources/:id/shares': { summary: 'Share a resource with one person, by email', description: 'Refused with 404 when no account has that email; accounts are never listed.', requestBody: { required: true, content: { 'application/json': { schema: { type: 'object', required: ['email'], properties: { email: { type: 'string' } } } } } } }, + 'DELETE /api/my-resources/:id/shares/:userId': { summary: 'Withdraw a share' }, 'GET /api/my-resources/:id/preview': { summary: 'Render a resource as pages for viewing in place', description: 'Returns the page count and a key; pages are served as PNG by the sibling route. Rendered once per version (updated_at and theme).' }, 'GET /api/my-resources/:id/preview/:page': { summary: 'One page of a resource preview, as PNG' }, 'GET /api/my-resources/theme-sample/:id/preview': { summary: 'Render a theme\'s sample deck as pages' }, diff --git a/test/clinical-mcp-session-lifecycle.test.js b/test/clinical-mcp-session-lifecycle.test.js index d0bbb5d2..855bb362 100644 --- a/test/clinical-mcp-session-lifecycle.test.js +++ b/test/clinical-mcp-session-lifecycle.test.js @@ -38,6 +38,9 @@ function loadClient(overrides = {}) { if (request === 'axios') return axios; return original.apply(this, arguments); }; + // These tests reason about one call after another; the pool is bounded to + // one here so the order they assert is the order that happens. + process.env.CLINICAL_ASSISTANT_MCP_CONCURRENCY = '1'; let client; try { client = require(target); } finally { Module._load = original; } return { client, calls, deletes: () => calls.filter(c => c.method === 'DELETE').map(c => c.config.headers['mcp-session-id']) }; diff --git a/test/mcp-client-concurrency.test.js b/test/mcp-client-concurrency.test.js new file mode 100644 index 00000000..47dff1e3 --- /dev/null +++ b/test/mcp-client-concurrency.test.js @@ -0,0 +1,34 @@ +// Library searches run a few at a time rather than one behind another, and +// the session is renewed before it expires. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const src = fs.readFileSync(path.join(__dirname, '..', 'src/utils/clinicalMcpClient.js'), 'utf8'); + +test('a bounded number of calls run at once; the rest wait their turn', async () => { + // The slot logic on its own, with the environment's default bound. + const slice = src.slice(src.indexOf('function acquireSlot'), src.indexOf('async function callMcpTool')); + const fn = new Function('MCP_CONCURRENCY', 'var _inFlight = 0, _waiting = [];' + slice + '; return { acquireSlot, releaseSlot, count: () => _inFlight, waiting: () => _waiting.length };')(3); + await fn.acquireSlot(); await fn.acquireSlot(); await fn.acquireSlot(); + assert.equal(fn.count(), 3); + let fourthStarted = false; + const fourth = fn.acquireSlot().then(() => { fourthStarted = true; }); + await new Promise(r => setTimeout(r, 5)); + assert.equal(fourthStarted, false, 'the fourth waits'); + assert.equal(fn.waiting(), 1); + fn.releaseSlot(); + await fourth; + assert.equal(fourthStarted, true, 'and runs when a slot frees'); + assert.equal(fn.count(), 3, 'the slot passed hands rather than being freed'); + fn.releaseSlot(); fn.releaseSlot(); fn.releaseSlot(); + assert.equal(fn.count(), 0); +}); + +test('the serial promise chain is gone, the session is kept warm, and the split is logged', () => { + assert.doesNotMatch(src, /_mcpCallQueue = queued\.catch/, 'no more one-at-a-time chain'); + assert.match(src, /var MCP_CONCURRENCY = positiveInt\(process\.env\.CLINICAL_ASSISTANT_MCP_CONCURRENCY, 3\)/); + assert.match(src, /setInterval\(function\(\) \{[\s\S]*?getMcpSession\(\)\.catch/, 'renewed on a timer'); + assert.match(src, /_warmTimer\.unref/, 'the timer never keeps the process alive'); + assert.match(src, /mcp ' \+ name \+ ': session=' \+ sessionMs \+ 'ms total='/); +}); diff --git a/test/resource-previews.test.js b/test/resource-previews.test.js index e49b4da7..27196ed4 100644 --- a/test/resource-previews.test.js +++ b/test/resource-previews.test.js @@ -24,9 +24,9 @@ test('previews are served for a resource and for a theme sample, and only to the assert.match(route, /router\.get\('\/my-resources\/:id\/preview\/:page', async/); assert.match(route, /router\.get\('\/my-resources\/theme-sample\/:id\/preview', async/); assert.match(route, /router\.get\('\/my-resources\/theme-sample\/:id\/preview\/:page', async/); - // The resource routes read with the owner in the WHERE, like every other route here. + // The resource routes read through the reader rule: the owner, or someone it is shared with. const preview = route.slice(route.indexOf("router.get('/my-resources/:id/preview'"), route.indexOf("// The sample deck of a theme")); - assert.equal((preview.match(/AND user_id = \?/g) || []).length, 2); + assert.equal((preview.match(/await readableResource\(req\.params\.id, req\.user\.id/g) || []).length, 2); // Rendered the way the download is: the same exporter, the same theme. assert.match(route, /documentExport\.render\(row\.markdown, row\.kind, format,\s*\{ images: figures, deck: row\.deck, theme: row\.theme, figureIds: figureIdList\(row\.image_ids\) \}\)/); assert.match(route, /previewPages\.cacheKey\(\['resource', row\.id, row\.updated_at, row\.theme \|\| '', row\.kind\]\)/); diff --git a/test/resource-sharing.test.js b/test/resource-sharing.test.js new file mode 100644 index 00000000..1340ef47 --- /dev/null +++ b/test/resource-sharing.test.js @@ -0,0 +1,41 @@ +// Sharing a resource: reading is extended to named people or to everyone +// signed in; writing never is. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const read = f => fs.readFileSync(path.join(__dirname, '..', f), 'utf8'); + +test('every read route goes through the reader rule; every write route still filters on the owner', () => { + const route = read('src/routes/myResources.js'); + assert.match(route, /WHERE r\.id = \? AND \(r\.user_id = \? OR r\.shared_with_all OR s\.user_id IS NOT NULL\)/); + for (const marker of ["router.get('/my-resources/:id'", "router.get('/my-resources/:id/export'", "router.get('/my-resources/:id/preview'", "router.get('/my-resources/:id/preview/:page'"]) { + const body = route.slice(route.indexOf(marker), route.indexOf('\n});', route.indexOf(marker))); + assert.match(body, /await readableResource\(req\.params\.id, req\.user\.id/, marker + ' reads through the rule'); + } + for (const marker of ["router.put('/my-resources/:id/theme'", "router.post('/my-resources/:id/refine'", "router.delete('/my-resources/:id'", "router.put('/my-resources/:id'"]) { + const body = route.slice(route.indexOf(marker), route.indexOf('\n});', route.indexOf(marker))); + assert.match(body, /AND user_id = \?/, marker + ' stays the owner\'s'); + assert.doesNotMatch(body, /readableResource/, marker + ' is not opened to readers'); + } + // The share list itself is the owner's, and adding by email never lists accounts. + for (const marker of ["router.get('/my-resources/:id/shares'", "router.put('/my-resources/:id/shares/all'", "router.post('/my-resources/:id/shares'", "router.delete('/my-resources/:id/shares/:userId'"]) { + const body = route.slice(route.indexOf(marker), route.indexOf('\n});', route.indexOf(marker))); + assert.match(body, /await ownedResource\(req\.params\.id, req\.user\.id\)/, marker + ' is owner-only'); + } + assert.match(route, /SELECT id, name, email FROM users WHERE email = \? AND disabled IS NOT TRUE/); + assert.match(route, /No account with that email on this site/); + assert.match(route, /ON CONFLICT DO NOTHING/, 'sharing twice is not an error'); +}); + +test('the list carries whose each row is, and the page offers changes only on your own', () => { + const route = read('src/routes/myResources.js'); + assert.match(route, /\(r\.user_id = \?\) AS owned, r\.shared_with_all/); + assert.match(route, /CASE WHEN r\.user_id = \? THEN NULL ELSE u\.name END AS shared_by_name/); + const js = read('public/js/myResources.js'); + assert.match(js, /if \(row\.owned === false\) \{ return wrap; \}/, 'a shared row has Preview and downloads, nothing else'); + assert.match(js, /library\.filter\(function \(row\) \{ return row\.owned !== false; \}\)/, 'Modify offers only your own'); + assert.match(js, /'Shared by ' \+ row\.shared_by_name/); + assert.match(js, /mr-share-all/); assert.match(js, /mr-share-email/); assert.match(js, /Withdraw the share/); + assert.match(read('migrations/1781300000000_resource-shares.js'), /PRIMARY KEY \(resource_id, user_id\)/); +});