From be002028deae857c2c1e454cc9a82abd10b491a3 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 15:53:32 -0300 Subject: [PATCH 01/13] feat(ocl): add $resolveReference client (global namespace) Client for OCL's $resolveReference: given a canonical URL (or relative OCL path), OCL answers which repo holds it. This replaces finding a repo by iterating source/collection listings and matching canonical_url client-side. Namespace is deliberately not supported. FHIR operations carry no namespace parameter, and a canonical URL is the same artifact regardless of which OCL namespace stores it -- so every resolution runs in OCL's global namespace. Namespace semantics (multi-tenancy, sandboxing) are an open discussion with the OCL team, not something to encode client-side yet. Behaviour notes, all verified against a live OCL instance: - $resolveReference is authenticated everywhere we probed, while the listing endpoints it replaces are public. Without a token the resolver is constructed disabled and callers keep their existing paths; 404/401/403 disable it for the rest of the process. - Batches go as one POST with the whole array; results are positional, so a count mismatch discards the batch rather than risk attributing a resolution to the wrong canonical. - The result carries the repo's own canonical_url and owner_type (the docs' example omits them); these are surfaced rather than echoing the request back. - url_registry_entry is surfaced even though every observed response carries null -- whether the URL Registry participates is exactly what the OCL-team discussion needs. The response-shape tests are built from a captured live payload, not from the documented example (which reports "Source Version" where OCL returns "Source"). Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-reference-resolver.test.js | 506 +++++++++++++++++++++++ tx/ocl/resolve/reference-resolver.js | 275 ++++++++++++ 2 files changed, 781 insertions(+) create mode 100644 tests/ocl/ocl-reference-resolver.test.js create mode 100644 tx/ocl/resolve/reference-resolver.js diff --git a/tests/ocl/ocl-reference-resolver.test.js b/tests/ocl/ocl-reference-resolver.test.js new file mode 100644 index 00000000..0640bf71 --- /dev/null +++ b/tests/ocl/ocl-reference-resolver.test.js @@ -0,0 +1,506 @@ +const { + OclReferenceResolver, + normalizeReference, + isOclRepoPath, + RESOLVE_PATH +} = require('../../tx/ocl/resolve/reference-resolver'); + +function silentLogger() { + return { info: jest.fn(), warn: jest.fn(), error: jest.fn() }; +} + +// One entry of OCL's $resolveReference response. +// +// Shaped from a real response captured against oclapi2.ips.hsl.org.br, NOT from +// the docs — the documented example shows only {type, short_code, url}, but the +// live payload carries canonical_url, owner_type, version and checksums, and +// reports type "Source" rather than "Source Version". +function oclEntry({ + resolved = true, + url = '/orgs/MS/sources/BRTabelaSUS/', + canonicalUrl = 'https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS', + owner = 'MS', + ownerType = 'Organization', + registryEntry = null, + referenceType = 'relative', + resolutionUrl = null, + request = null +} = {}) { + return { + reference_type: referenceType, + timestamp: '2026-07-15T17:25:44.682776', + resolved, + request, + resolution_url: resolutionUrl, + url_registry_entry: registryEntry, + result: resolved + ? { + short_code: url.split('/').filter(Boolean).pop(), + name: url.split('/').filter(Boolean).pop(), + url, + owner, + owner_type: ownerType, + owner_url: `/orgs/${owner}/`, + version: 'HEAD', + source_type: 'Dictionary', + canonical_url: canonicalUrl, + type: 'Source', + checksums: { standard: '282d3c8ce440b8a03698196967042a08', smart: '90599db3f6da397c1af26baaf9467eb1' } + } + : null + }; +} + +// Echoes one result per submitted ref, tagging the url so ordering is assertable. +function echoClient() { + return { + post: jest.fn(async (path, body) => ({ + data: body.map(ref => { + const url = typeof ref === 'string' ? ref : ref.url; + return oclEntry({ url: `/orgs/X/sources/${url}/`, request: ref }); + }) + })) + }; +} + +function makeResolver({ httpClient, logger, token = 'Token abc' } = {}) { + return new OclReferenceResolver({ + httpClient: httpClient || echoClient(), + logger: logger || silentLogger(), + token + }); +} + +function httpError(status, data) { + const error = new Error(`Request failed with status code ${status}`); + error.response = { status, data }; + return error; +} + +describe('isOclRepoPath', () => { + it.each([ + ['/orgs/CIEL/sources/CIEL/'], + ['/orgs/CIEL/sources/CIEL/HEAD/'], + ['/orgs/CIEL/collections/C/'], + // The point: user-owned repos are real and must not be filtered out. + ['/users/joe/sources/S/'], + [' /users/joe/sources/S/ '] + ])('accepts %s', input => { + expect(isOclRepoPath(input)).toBe(true); + }); + + it.each([ + ['http://loinc.org'], + ['/sources/S/'], + ['/orgs/'], + ['/orgs/CIEL'], + ['/groups/x/sources/S/'], + [''], + [null], + [undefined] + ])('rejects %p', input => { + expect(isOclRepoPath(input)).toBe(false); + }); +}); + +describe('normalizeReference', () => { + it('passes a relative path string through as a string', () => { + expect(normalizeReference('/orgs/CIEL/sources/CIEL/')).toBe('/orgs/CIEL/sources/CIEL/'); + }); + + it('keeps the expanded object fields', () => { + expect(normalizeReference({ + url: 'http://hl7.org/fhir/CodeSystem/x', + version: '0.8', + code: '1948', + resourceType: 'Mapping' + })).toEqual({ + url: 'http://hl7.org/fhir/CodeSystem/x', + version: '0.8', + code: '1948', + resourceType: 'Mapping' + }); + }); + + it('never emits a namespace field (global-namespace only by design)', () => { + const body = normalizeReference({ url: '/orgs/A/sources/S/', namespace: '/orgs/A/' }); + expect(body).not.toHaveProperty('namespace'); + }); + + it('drops null and undefined fields', () => { + expect(normalizeReference({ url: 'x', version: null, code: undefined })).toEqual({ url: 'x' }); + }); + + it.each([[''], [' ']])('rejects the empty string %p', input => { + expect(() => normalizeReference(input)).toThrow(/cannot be empty/); + }); + + it.each([[{}], [{ url: '' }], [{ url: null }]])('rejects object %p without a url', input => { + expect(() => normalizeReference(input)).toThrow(/requires a url/); + }); + + it.each([[null], [undefined], [123], [[]]])('rejects %p', input => { + expect(() => normalizeReference(input)).toThrow(/Invalid OCL reference/); + }); +}); + +describe('OclReferenceResolver construction', () => { + it('requires an http client', () => { + expect(() => new OclReferenceResolver({ token: 'Token abc' })).toThrow(/requires an http client/); + }); + + // $resolveReference is auth-gated on every instance probed, while the listing + // endpoints it replaces are public — a tokenless call is a guaranteed 401. + it('stays disabled without a token and issues no request', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient, token: null }); + + expect(resolver.isEnabled()).toBe(false); + expect(resolver.disabledReason).toBe('no token configured'); + await expect(resolver.resolveReferences(['/orgs/A/sources/S/'])).resolves.toBeNull(); + expect(httpClient.post).not.toHaveBeenCalled(); + }); + + it('is enabled with a token', () => { + expect(makeResolver().isEnabled()).toBe(true); + }); +}); + +describe('OclReferenceResolver resolution', () => { + it('resolves a single relative reference', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/CIEL/sources/CIEL/' })] })) + }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve('/orgs/CIEL/sources/CIEL/'); + + expect(result.resolved).toBe(true); + expect(result.repoUrl).toBe('/orgs/CIEL/sources/CIEL/'); + expect(httpClient.post).toHaveBeenCalledWith(RESOLVE_PATH, ['/orgs/CIEL/sources/CIEL/']); + }); + + it('resolves an expanded object reference with a version', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ referenceType: 'canonical' })] })) + }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve({ url: 'http://hl7.org/fhir/CodeSystem/x', version: '0.8' }); + + expect(result.resolved).toBe(true); + expect(result.referenceType).toBe('canonical'); + expect(httpClient.post).toHaveBeenCalledWith( + RESOLVE_PATH, + [{ url: 'http://hl7.org/fhir/CodeSystem/x', version: '0.8' }] + ); + }); + + // Verbatim response captured from oclapi2.ips.hsl.org.br for + // POST /$resolveReference/ ["/orgs/MS/sources/BRTabelaSUS/concepts/1948/"]. + // Guards against the doc's example, which omits most of these fields. + it('handles a real relative-reference response, concept and all', async () => { + const live = [{ + reference_type: 'relative', + resolved: true, + timestamp: '2026-07-15T17:25:44.682776', + request: '/orgs/MS/sources/BRTabelaSUS/concepts/1948/', + resolution_url: '/orgs/MS/sources/BRTabelaSUS/', + url_registry_entry: null, + result: { + short_code: 'BRTabelaSUS', + name: 'BRTabelaSUS', + url: '/orgs/MS/sources/BRTabelaSUS/', + owner: 'MS', + owner_type: 'Organization', + owner_url: '/orgs/MS/', + version: 'HEAD', + created_at: '2025-10-24T12:41:29.742565Z', + id: 'BRTabelaSUS', + source_type: 'Dictionary', + updated_at: '2026-06-22T15:25:14.276339Z', + canonical_url: 'https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS', + type: 'Source', + checksums: { standard: '282d3c8ce440b8a03698196967042a08', smart: '90599db3f6da397c1af26baaf9467eb1' } + } + }]; + const httpClient = { post: jest.fn(async () => ({ data: live })) }; + const resolver = makeResolver({ httpClient }); + + const r = await resolver.resolve('/orgs/MS/sources/BRTabelaSUS/concepts/1948/'); + + expect(r.resolved).toBe(true); + expect(r.repoUrl).toBe('/orgs/MS/sources/BRTabelaSUS/'); + expect(r.referenceType).toBe('relative'); + // OCL strips the concept: the reference points at a concept, the repo is the source. + expect(r.resolutionUrl).toBe('/orgs/MS/sources/BRTabelaSUS/'); + expect(r.canonical).toBe('https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS'); + expect(r.ownerType).toBe('Organization'); + expect(r.registryEntry).toBeNull(); + // The raw result stays available for anything we don't surface. + expect(r.result.checksums.standard).toBe('282d3c8ce440b8a03698196967042a08'); + }); + + it('surfaces the repo canonical_url rather than echoing the requested spelling', async () => { + const httpClient = { post: jest.fn(async () => ({ data: [oclEntry()] })) }; + const resolver = makeResolver({ httpClient }); + + // Asked with a different spelling (http) than the repo's own canonical (https). + const r = await resolver.resolve('http://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS'); + + expect(r.canonical).toBe('https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS'); + }); + + it('resolves a user-owned repo (an /orgs/-only filter would drop these)', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/users/joe/sources/S/' })] })) + }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve('/users/joe/sources/S/'); + + expect(result.repoUrl).toBe('/users/joe/sources/S/'); + expect(isOclRepoPath(result.repoUrl)).toBe(true); + }); + + it('returns an empty array and issues no request for no references', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + await expect(resolver.resolveReferences([])).resolves.toEqual([]); + expect(httpClient.post).not.toHaveBeenCalled(); + }); + + it('reports resolved:false without throwing (OCL returns 200, not an error)', async () => { + const httpClient = { post: jest.fn(async () => ({ data: [oclEntry({ resolved: false })] })) }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve('/orgs/nope/sources/nope/'); + + expect(result.resolved).toBe(false); + expect(result.repoUrl).toBeNull(); + }); + + it('treats a resolved flag with no result url as unresolved', async () => { + const httpClient = { post: jest.fn(async () => ({ data: [{ resolved: true, result: null }] })) }; + const resolver = makeResolver({ httpClient }); + + await expect(resolver.resolve('/orgs/A/sources/S/')).resolves.toMatchObject({ resolved: false }); + }); + + it.each([[null], ['oops'], [42]])('treats a malformed result entry %p as unresolved', async entry => { + const httpClient = { post: jest.fn(async () => ({ data: [entry] })) }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve('/orgs/A/sources/S/'); + + expect(result.resolved).toBe(false); + expect(result.repoUrl).toBeNull(); + }); + + it('tolerates a non-array response body', async () => { + const httpClient = { post: jest.fn(async () => ({ data: oclEntry({ url: '/orgs/A/sources/S/' }) })) }; + const resolver = makeResolver({ httpClient }); + + const result = await resolver.resolve('/orgs/A/sources/S/'); + + expect(result.repoUrl).toBe('/orgs/A/sources/S/'); + }); + + it('reads camelCase result fields as a fallback', async () => { + const httpClient = { + post: jest.fn(async () => ({ + data: [{ + resolved: true, + result: { url: '/orgs/A/sources/S/', canonicalUrl: 'http://a.org/cs', ownerType: 'User' } + }] + })) + }; + const resolver = makeResolver({ httpClient }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.canonical).toBe('http://a.org/cs'); + expect(r.ownerType).toBe('User'); + }); + + it("prefers OCL's own request echo over the submitted body", async () => { + const httpClient = { + post: jest.fn(async () => ({ + data: [{ ...oclEntry(), request: { url: 'echoed-by-ocl' } }] + })) + }; + const resolver = makeResolver({ httpClient }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.request).toEqual({ url: 'echoed-by-ocl' }); + }); + + it('accepts a single non-array reference in resolveReferences', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + const results = await resolver.resolveReferences('a'); + + expect(results).toHaveLength(1); + expect(results[0].repoUrl).toBe('/orgs/X/sources/a/'); + }); + + it('resolve() returns null when the resolver is unavailable', async () => { + const resolver = makeResolver({ token: null }); + await expect(resolver.resolve('/orgs/A/sources/S/')).resolves.toBeNull(); + }); + + it('treats a null response data as a misaligned batch', async () => { + const httpClient = { post: jest.fn(async () => ({ data: null })) }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + const result = await resolver.resolve('/orgs/A/sources/S/'); + + expect(result.resolved).toBe(false); + expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/misaligned/)); + }); +}); + +describe('OclReferenceResolver batching', () => { + it('sends the whole batch as ONE POST and preserves caller order', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + const results = await resolver.resolveReferences(['a', { url: 'b', version: '1.0' }, 'c']); + + expect(httpClient.post).toHaveBeenCalledTimes(1); + expect(httpClient.post.mock.calls[0][1]).toEqual(['a', { url: 'b', version: '1.0' }, 'c']); + expect(results.map(r => r.repoUrl)).toEqual([ + '/orgs/X/sources/a/', + '/orgs/X/sources/b/', + '/orgs/X/sources/c/' + ]); + }); + + it('discards a batch whose result count does not match the request count', async () => { + // Never let result[0] be attributed to the wrong canonical. + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/only/' })] })) + }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + const results = await resolver.resolveReferences(['a', 'b']); + + expect(results).toHaveLength(2); + expect(results.every(r => r.resolved === false)).toBe(true); + expect(logger.error).toHaveBeenCalledWith( + expect.stringMatching(/returned 1 result\(s\) for 2 reference\(s\).*misaligned/) + ); + }); +}); + +describe('OclReferenceResolver caching', () => { + it('serves a repeat reference from cache', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + const first = await resolver.resolve('/orgs/A/sources/S/'); + const second = await resolver.resolve('/orgs/A/sources/S/'); + + expect(httpClient.post).toHaveBeenCalledTimes(1); + expect(second).toEqual(first); + }); + + it('distinguishes references that differ only by a field', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + await resolver.resolve({ url: 'S', code: '1' }); + await resolver.resolve({ url: 'S', code: '2' }); + + expect(httpClient.post).toHaveBeenCalledTimes(2); + }); + + it('mixes cached and uncached references in one call, POSTing only the misses', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + await resolver.resolve('a'); + httpClient.post.mockClear(); + + const results = await resolver.resolveReferences(['a', 'b']); + + expect(httpClient.post).toHaveBeenCalledTimes(1); + expect(httpClient.post.mock.calls[0][1]).toEqual(['b']); + expect(results.map(r => r.repoUrl)).toEqual(['/orgs/X/sources/a/', '/orgs/X/sources/b/']); + }); + + it('does not cache a failed resolution', async () => { + const httpClient = { + post: jest + .fn() + .mockRejectedValueOnce(httpError(400, { detail: 'bad' })) + .mockResolvedValueOnce({ data: [oclEntry({ url: '/orgs/A/sources/S/' })] }) + }; + const resolver = makeResolver({ httpClient, logger: silentLogger() }); + + const first = await resolver.resolve('/orgs/A/sources/S/'); + const second = await resolver.resolve('/orgs/A/sources/S/'); + + expect(first.resolved).toBe(false); + expect(second.resolved).toBe(true); + expect(httpClient.post).toHaveBeenCalledTimes(2); + }); +}); + +describe('OclReferenceResolver error handling', () => { + it('disables itself and falls back when the endpoint is missing (404)', async () => { + const httpClient = { post: jest.fn().mockRejectedValue(httpError(404)) }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + await expect(resolver.resolveReferences(['a'])).resolves.toBeNull(); + expect(resolver.isEnabled()).toBe(false); + expect(resolver.disabledReason).toBe('endpoint not implemented (404)'); + expect(logger.info).toHaveBeenCalledWith(expect.stringMatching(/not available/)); + + // Stays disabled: no further requests. + await expect(resolver.resolveReferences(['b'])).resolves.toBeNull(); + expect(httpClient.post).toHaveBeenCalledTimes(1); + }); + + it.each([[401], [403]])('disables itself and falls back on %i', async status => { + const httpClient = { post: jest.fn().mockRejectedValue(httpError(status)) }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + await expect(resolver.resolveReferences(['a'])).resolves.toBeNull(); + expect(resolver.isEnabled()).toBe(false); + expect(resolver.disabledReason).toBe(`not authorised (${status})`); + expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/credentials/)); + }); + + // A 400 is our bug, not the instance's — log it loudly but stay enabled, since a + // later well-formed batch may be fine. + it('reports a 400 without disabling', async () => { + const httpClient = { post: jest.fn().mockRejectedValue(httpError(400, { detail: 'malformed' })) }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + const results = await resolver.resolveReferences(['a', 'b']); + + expect(results).toHaveLength(2); + expect(results.every(r => r.resolved === false)).toBe(true); + expect(resolver.isEnabled()).toBe(true); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('malformed')); + }); + + it('falls back on a network timeout without disabling', async () => { + const httpClient = { post: jest.fn().mockRejectedValue(new Error('timeout of 30000ms exceeded')) }; + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); + + await expect(resolver.resolveReferences(['a'])).resolves.toBeNull(); + expect(resolver.isEnabled()).toBe(true); + expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/timeout/)); + }); +}); diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js new file mode 100644 index 00000000..4d2372a9 --- /dev/null +++ b/tx/ocl/resolve/reference-resolver.js @@ -0,0 +1,275 @@ +// Client for OCL's $resolveReference operation. +// +// Answers "which OCL repo holds this canonical URL?" authoritatively, so callers +// stop iterating source/collection listings to find a matching canonical_url. See +// https://docs.openconceptlab.org/en/latest/oclapi/apireference/resolveReference.html +// +// Namespace is deliberately NOT supported: FHIR operations carry no namespace +// parameter, so every resolution here runs in OCL's global namespace. Namespace +// semantics (multi-tenant discrimination, sandboxing) are an open discussion with +// the OCL team, not something to encode client-side yet. +// +// Plain .js rather than tx/ocl's .cjs+stub convention: jest's collectCoverageFrom +// globs **/*.js only, so a .cjs module would be invisible to coverage. + +const RESOLVE_PATH = '/$resolveReference/'; + +// OCL repos are owned by an org OR a user. Matching only /orgs/ silently drops +// every user-owned repo. +const REPO_PATH_PATTERN = /^\/(orgs|users)\/[^/]+\//; + +// Reference object fields forwarded to OCL. `namespace` is intentionally absent. +const BODY_FIELDS = [ + 'url', + 'version', + 'code', + 'display', + 'id', + 'filter', + 'cascade', + 'includeExclude', + 'resourceType' +]; + +/** + * True for a relative OCL repo path under either owner type, e.g. + * `/orgs/CIEL/sources/CIEL/` or `/users/joe/sources/S/`. + */ +function isOclRepoPath(value) { + return REPO_PATH_PATTERN.test(String(value == null ? '' : value).trim()); +} + +/** + * Accepts either a relative/canonical URL string or an expanded reference object, + * returning the request-body form OCL expects. + */ +function normalizeReference(ref) { + if (typeof ref === 'string') { + const url = ref.trim(); + if (!url) { + throw new Error('OCL reference string cannot be empty'); + } + return url; + } + + if (!ref || typeof ref !== 'object' || Array.isArray(ref)) { + throw new Error(`Invalid OCL reference: expected a string or object, got ${typeof ref}`); + } + + const url = String(ref.url == null ? '' : ref.url).trim(); + if (!url) { + throw new Error('OCL reference object requires a url'); + } + + const body = {}; + for (const field of BODY_FIELDS) { + if (ref[field] !== undefined && ref[field] !== null) { + body[field] = ref[field]; + } + } + return body; +} + +function cacheKey(body) { + return typeof body === 'string' ? body : JSON.stringify(body); +} + +function unresolved(request) { + return { + resolved: false, + repoUrl: null, + canonical: null, + ownerType: null, + resolutionUrl: null, + registryEntry: null, + referenceType: null, + request, + result: null + }; +} + +function normalizeResult(entry, request) { + if (!entry || typeof entry !== 'object') { + return unresolved(request); + } + + const result = entry.result && typeof entry.result === 'object' ? entry.result : null; + const repoUrl = result && result.url ? result.url : null; + + return { + resolved: Boolean(entry.resolved) && Boolean(repoUrl), + repoUrl, + // OCL returns the repo's own canonical_url and owner_type (richer than the + // documented example). Prefer them over echoing the request back: they are + // authoritative, the request is just whatever spelling the caller used. + canonical: result ? (result.canonical_url || result.canonicalUrl || null) : null, + ownerType: result ? (result.owner_type || result.ownerType || null) : null, + resolutionUrl: entry.resolution_url || null, + // Kept even though every observed response so far carries null: whether a URL + // Registry entry was involved is exactly what the OCL-team discussion needs. + registryEntry: entry.url_registry_entry || null, + referenceType: entry.reference_type || null, + request: entry.request === undefined ? request : entry.request, + result + }; +} + +class OclReferenceResolver { + #httpClient; + #logger; + #cache = new Map(); + #enabled; + #disabledReason = null; + + /** + * @param {object} options + * @param {object} options.httpClient - axios instance from createOclHttpClient + * @param {string} [options.token] - when absent the resolver stays disabled: + * $resolveReference is authenticated on every OCL instance probed, while the + * listing endpoints it replaces are public, so a tokenless call is a + * guaranteed 401 + * @param {object} [options.logger] + */ + constructor({ httpClient, token = null, logger = console } = {}) { + if (!httpClient) { + throw new Error('OCL reference resolver requires an http client'); + } + + this.#httpClient = httpClient; + this.#logger = logger; + this.#enabled = Boolean(token); + if (!this.#enabled) { + this.#disabledReason = 'no token configured'; + } + } + + isEnabled() { + return this.#enabled; + } + + get disabledReason() { + return this.#disabledReason; + } + + #disable(reason) { + this.#enabled = false; + this.#disabledReason = reason; + } + + /** + * Resolve one reference. Returns null when the resolver is unavailable, so the + * caller falls back to its existing path. + */ + async resolve(ref) { + const results = await this.resolveReferences([ref]); + return results ? results[0] : null; + } + + /** + * Resolve many references in one POST. Results come back in the caller's order. + * + * @returns {Promise} null when the resolver is unavailable (use fallback) + */ + async resolveReferences(refs) { + if (!this.#enabled) { + return null; + } + + const list = Array.isArray(refs) ? refs : [refs]; + if (list.length === 0) { + return []; + } + + const bodies = list.map(normalizeReference); + const output = new Array(bodies.length).fill(null); + const misses = []; + + bodies.forEach((body, index) => { + const key = cacheKey(body); + if (this.#cache.has(key)) { + output[index] = this.#cache.get(key); + } else { + misses.push({ body, index }); + } + }); + + if (misses.length === 0) { + return output; + } + + let response; + try { + response = await this.#httpClient.post(RESOLVE_PATH, misses.map(m => m.body)); + } catch (error) { + return this.#handleError(error, misses, output); + } + + const payload = Array.isArray(response?.data) + ? response.data + : response?.data == null + ? [] + : [response.data]; + + // Results are positional. On a count mismatch we cannot know which result + // belongs to which reference, so treat everything as unresolved rather than + // silently attributing a resolution to the wrong canonical. + if (payload.length !== misses.length) { + this.#logger.error( + `[OCL] $resolveReference returned ${payload.length} result(s) for ${misses.length} reference(s); discarding to avoid misaligned results` + ); + for (const { body, index } of misses) { + output[index] = unresolved(body); + } + return output; + } + + misses.forEach(({ body, index }, position) => { + const value = normalizeResult(payload[position], body); + this.#cache.set(cacheKey(body), value); + output[index] = value; + }); + + return output; + } + + #handleError(error, misses, output) { + const status = error?.response?.status; + + if (status === 404) { + this.#disable('endpoint not implemented (404)'); + this.#logger.info( + `[OCL] $resolveReference is not available on this instance (404); using listing search instead` + ); + return null; + } + + if (status === 401 || status === 403) { + this.#disable(`not authorised (${status})`); + this.#logger.warn( + `[OCL] $resolveReference rejected our credentials (${status}); using listing search instead` + ); + return null; + } + + if (status === 400) { + // Our request body is wrong — a bug on this side. Don't disable: a later, + // well-formed batch may be fine. + const detail = error?.response?.data?.detail || error.message; + this.#logger.error(`[OCL] $resolveReference rejected the request body: ${detail}`); + for (const { body, index } of misses) { + output[index] = unresolved(body); + } + return output; + } + + this.#logger.warn(`[OCL] $resolveReference failed: ${error.message}`); + return null; + } +} + +module.exports = { + OclReferenceResolver, + normalizeReference, + isOclRepoPath, + RESOLVE_PATH +}; From 9097c9f80b753ed0cba56599f7099c376ac759b5 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 15:53:32 -0300 Subject: [PATCH 02/13] feat(ocl): resolve ConceptMap source canonicals via $resolveReference #candidateSourceUrls answered "which OCL repo holds this canonical?" by heuristic: derive a search token from the URL, page through /orgs/{org}/sources/?q=..., and match canonical_url on whatever the search surfaces. Ask $resolveReference first; any failure falls back to that search, so behaviour without a token is unchanged. Two prerequisites are included because the resolver exposed them: - searchConceptMaps lower-cased every param value. Every consumer compared through #norm() (which lower-cases anyway) so nothing noticed, and the text search tolerated it -- but $resolveReference matches the canonical exactly, so every lookup silently failed to resolve until this was fixed. - Candidate repo paths were filtered with startsWith('/orgs/'), silently dropping user-owned repos (/users/{user}/...), which the resolver legitimately returns. The repo's own canonical_url from the resolve result is recorded in the canonical<->repo bookkeeping instead of echoing the caller's spelling. Which path served a lookup is logged either way -- resolver and search return the same thing, so success was otherwise unobservable. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cm-provider.test.js | 110 ++++++++++++++++++++++++++++++ tx/ocl/cm-ocl.cjs | 84 +++++++++++++++++++++-- 2 files changed, 187 insertions(+), 7 deletions(-) diff --git a/tests/ocl/ocl-cm-provider.test.js b/tests/ocl/ocl-cm-provider.test.js index a1e2fc4a..c7b0006b 100644 --- a/tests/ocl/ocl-cm-provider.test.js +++ b/tests/ocl/ocl-cm-provider.test.js @@ -411,3 +411,113 @@ describe('OCLConceptMapProvider', () => { }); }); }); + +// --------------------------------------------------------------- +// $resolveReference integration +// --------------------------------------------------------------- +describe('OCLConceptMapProvider $resolveReference integration', () => { + const { OclReferenceResolver } = require('../../tx/ocl/resolve/reference-resolver'); + + const SEARCH_ENDPOINT = '/orgs/TestOrg/sources/'; + + function makeProvider({ token = 'Token abc', post } = {}) { + const provider = new OCLConceptMapProvider({ org: 'TestOrg', token }); + const httpClient = { + get: jest.fn(async () => ({ data: [] })), + post: post || jest.fn(async () => ({ data: [] })) + }; + // The provider captures httpClient at construction, so rebuild the resolver + // on the mock the same way the other tests swap httpClient. + provider.httpClient = httpClient; + provider.referenceResolver = new OclReferenceResolver({ + httpClient, + token, + logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() } + }); + return { provider, httpClient }; + } + + function resolveReferenceReply(url) { + return jest.fn(async () => ({ + data: [ + { + reference_type: 'canonical', + resolved: true, + request: null, + resolution_url: url, + url_registry_entry: null, + result: { type: 'Source', short_code: 'S', url, canonical_url: 'http://x.org/cs', owner_type: 'Organization' } + } + ] + })); + } + + function getPaths(httpClient) { + return httpClient.get.mock.calls.map(call => call[0]); + } + + const searchParams = [{ name: 'source-system', value: 'http://x.org/cs' }]; + + it('uses the resolved repo and skips the heuristic source search', async () => { + const { provider, httpClient } = makeProvider({ + post: resolveReferenceReply('/users/joe/sources/S/') + }); + + await provider.searchConceptMaps(searchParams); + + expect(httpClient.post).toHaveBeenCalledTimes(1); + // The authoritative answer makes the q= text search unnecessary. + expect(getPaths(httpClient)).not.toContain(SEARCH_ENDPOINT); + // ...and a user-owned repo survives, which an /orgs/-only filter would drop. + expect(getPaths(httpClient)).toContain('/users/joe/sources/S/concepts/'); + }); + + it('falls back to the source search when no token is configured', async () => { + const { provider, httpClient } = makeProvider({ token: null }); + + await provider.searchConceptMaps(searchParams); + + expect(httpClient.post).not.toHaveBeenCalled(); + expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); + }); + + it('falls back to the source search when OCL cannot resolve the canonical', async () => { + const post = jest.fn(async () => ({ + data: [{ reference_type: 'canonical', resolved: false, result: null }] + })); + const { provider, httpClient } = makeProvider({ post }); + + await provider.searchConceptMaps(searchParams); + + expect(post).toHaveBeenCalledTimes(1); + expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); + }); + + it('falls back to the source search when $resolveReference is unavailable', async () => { + const error = new Error('Request failed with status code 404'); + error.response = { status: 404 }; + const { provider, httpClient } = makeProvider({ post: jest.fn().mockRejectedValue(error) }); + + await provider.searchConceptMaps(searchParams); + + expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); + }); + + // Regression: searchConceptMaps used to lower-case every param value. The old + // text search tolerated it (#norm lower-cases anyway), but $resolveReference + // matches the canonical exactly, so a lower-cased URL never resolved. + it('sends the canonical to $resolveReference with its original casing', async () => { + const { provider, httpClient } = makeProvider({ + post: resolveReferenceReply('/orgs/Mangara/sources/S/') + }); + + await provider.searchConceptMaps([ + { name: 'source-system', value: 'https://mangara.hsl.org.br/fhir/CodeSystem/AlcoolSPA_uso_Mangara' } + ]); + + expect(httpClient.post).toHaveBeenCalledTimes(1); + expect(httpClient.post.mock.calls[0][1]).toEqual([ + 'https://mangara.hsl.org.br/fhir/CodeSystem/AlcoolSPA_uso_Mangara' + ]); + }); +}); diff --git a/tx/ocl/cm-ocl.cjs b/tx/ocl/cm-ocl.cjs index d726ef0a..f5cf3f3f 100644 --- a/tx/ocl/cm-ocl.cjs +++ b/tx/ocl/cm-ocl.cjs @@ -3,6 +3,7 @@ const { ConceptMap } = require('../library/conceptmap'); const { PAGE_SIZE } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); const { fetchAllPages, extractItemsAndNext } = require('./http/pagination'); +const { OclReferenceResolver, isOclRepoPath } = require('./resolve/reference-resolver'); const DEFAULT_MAX_SEARCH_PAGES = 10; @@ -22,6 +23,14 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { this._sourceCandidatesCache = new Map(); this._sourceUrlsByCanonical = new Map(); this._canonicalBySourceUrl = new Map(); + + // Asks OCL which repo holds a canonical instead of guessing via text search. + // Disabled unless a token is configured, in which case #candidateSourceUrls + // keeps using its existing search. + this.referenceResolver = new OclReferenceResolver({ + httpClient: this.httpClient, + token: options.token || null + }); } assignIds(ids) { @@ -107,8 +116,12 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { async searchConceptMaps(searchParams, _elements) { this._validateSearchParams(searchParams); + // Keep the canonical exactly as supplied. Lower-casing it was harmless while + // every consumer compared through #norm() (which lower-cases anyway), but + // $resolveReference matches the canonical exactly, and OCL has no + // .../fhir/codesystem/x -- only .../fhir/CodeSystem/X. const params = Object.fromEntries( - searchParams.map(({ name, value }) => [name, String(value).toLowerCase()]) + searchParams.map(({ name, value }) => [name, String(value)]) ); const sourceSystem = params['source-system'] || params.source || null; const targetSystem = params['target-system'] || params.target || null; @@ -131,7 +144,7 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { async #collectMappingsForSearch(sourceSystem, targetSystem) { const systemUrl = sourceSystem || targetSystem; const candidates = await this.#candidateSourceUrls(systemUrl); - const sourcePaths = candidates.filter(s => String(s || '').startsWith('/orgs/')); + const sourcePaths = candidates.filter(isOclRepoPath); if (sourcePaths.length === 0) { return []; @@ -283,7 +296,7 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { const targetCandidates = await this.#candidateSourceUrls(targetSystem); const mappings = []; - const sourcePaths = sourceCandidates.filter(s => String(s || '').startsWith('/orgs/')); + const sourcePaths = sourceCandidates.filter(isOclRepoPath); if (sourceCode && sourcePaths.length > 0) { for (const sourcePath of sourcePaths) { @@ -515,9 +528,16 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { } } - const discovered = await this.#resolveSourceCandidatesFromOcl(systemUrl); - for (const item of discovered) { - result.add(item); + // Ask OCL which repo holds this canonical. An authoritative answer makes the + // heuristic text search below unnecessary; without one we fall back to it. + const resolved = await this.#resolveViaReferenceResolver(systemUrl); + if (resolved) { + result.add(resolved); + } else { + const discovered = await this.#resolveSourceCandidatesFromOcl(systemUrl); + for (const item of discovered) { + result.add(item); + } } const out = Array.from(result); @@ -525,6 +545,56 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { return out; } + /** + * Resolve a canonical to its OCL repo via $resolveReference. + * Returns null when the resolver is disabled (no token), the endpoint is + * unavailable, or OCL cannot resolve the canonical -- in every case the caller + * falls back to the existing search. + */ + async #resolveViaReferenceResolver(systemUrl) { + // Say why once, rather than per lookup: for a tokenless deployment this is the + // expected steady state, not an error. + if (!this.referenceResolver.isEnabled()) { + if (!this._resolverDisabledLogged) { + this._resolverDisabledLogged = true; + console.log( + `[OCL] $resolveReference not in use (${this.referenceResolver.disabledReason}); using source search` + ); + } + return null; + } + + let resolved; + try { + resolved = await this.referenceResolver.resolve(systemUrl); + } catch (error) { + console.warn(`[OCL] $resolveReference lookup failed for ${systemUrl}: ${error.message}`); + return null; + } + + if (!resolved?.resolved || !resolved.repoUrl) { + console.log(`[OCL] $resolveReference did not resolve ${systemUrl}; falling back to source search`); + return null; + } + + // Logged on success too: resolver and search return the same thing, so without + // this there is no way -- from outside or from the logs -- to tell which path ran. + console.log(`[OCL] $resolveReference resolved ${systemUrl} -> ${resolved.repoUrl}`); + + // Keep the canonical<->repo caches coherent with the search path's bookkeeping. + // Index under what was asked, but record OCL's own canonical_url as the repo's + // canonical: echoing the request back would store whatever spelling the caller + // used rather than the repo's actual identity. + const canonicalKey = this.#norm(systemUrl); + if (!this._sourceUrlsByCanonical.has(canonicalKey)) { + this._sourceUrlsByCanonical.set(canonicalKey, new Set()); + } + this._sourceUrlsByCanonical.get(canonicalKey).add(resolved.repoUrl); + this._canonicalBySourceUrl.set(this.#norm(resolved.repoUrl), resolved.canonical || systemUrl); + + return resolved.repoUrl; + } + async #resolveSourceCandidatesFromOcl(systemUrl) { const endpoint = this.org ? `/orgs/${encodeURIComponent(this.org)}/sources/` : '/sources/'; const query = this.#queryTokenFromSystem(systemUrl); @@ -572,7 +642,7 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { } const sourcePath = String(sourceUrl || '').trim(); - if (!sourcePath.startsWith('/orgs/')) { + if (!isOclRepoPath(sourcePath)) { continue; } From aa9fadc1b61a992327b73bf7cb6549ac1188aa96 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 15:53:32 -0300 Subject: [PATCH 03/13] feat(ocl): resolve ValueSet collections and compose sources via $resolveReference Covers the ValueSet provider's two canonical->repo points: - #findCollectionByCanonical: fetchValueSet for a canonical the enumeration did not bring in searched every org's /collections/ with a q= text token, matching canonical_url client-side. Ask $resolveReference first; a result that is not a collection, a version mismatch, or any failure falls through to the search. - compose sources: #buildCollectionSources resolved each source's canonical with one sequential GET per source. #primeSourceCanonicalsBatch resolves all of them in a single $resolveReference POST and seeds sourceCanonicalCache; anything unresolved falls back to the per-source GET, so this only saves round trips, never changes results. Both are no-ops without a token, matching the ConceptMap behaviour. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-vs-resolve-reference.test.js | 109 +++++++++++++++++ tx/ocl/vs-ocl.cjs | 129 +++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 tests/ocl/ocl-vs-resolve-reference.test.js diff --git a/tests/ocl/ocl-vs-resolve-reference.test.js b/tests/ocl/ocl-vs-resolve-reference.test.js new file mode 100644 index 00000000..af38d6f8 --- /dev/null +++ b/tests/ocl/ocl-vs-resolve-reference.test.js @@ -0,0 +1,109 @@ +// $resolveReference integration in the ValueSet provider: resolve a ValueSet +// canonical to its collection without searching every org's collection listing. + +const { OCLValueSetProvider } = require('../../tx/ocl/vs-ocl'); +const { OclReferenceResolver } = require('../../tx/ocl/resolve/reference-resolver'); + +function silentLogger() { + return { info: jest.fn(), warn: jest.fn(), error: jest.fn() }; +} + +// One $resolveReference entry resolving a canonical to a collection. +function collectionResolve(repoUrl, canonical) { + return { + reference_type: 'canonical', + resolved: true, + resolution_url: repoUrl, + url_registry_entry: null, + result: { + url: repoUrl, + canonical_url: canonical, + owner_type: 'Organization', + type: 'Collection' + } + }; +} + +function makeProvider({ post, get } = {}) { + const provider = new OCLValueSetProvider({ org: 'MS', token: 'Token x' }); + const httpClient = { + get: get || jest.fn(async () => ({ data: [] })), + post: post || jest.fn(async () => ({ data: [] })) + }; + provider.httpClient = httpClient; + provider.referenceResolver = new OclReferenceResolver({ + httpClient, token: 'Token x', logger: silentLogger() + }); + return { provider, httpClient }; +} + +describe('vs-ocl $resolveReference: collection resolution', () => { + it('resolves an unenumerated collection via $resolveReference and skips the per-org search', async () => { + const collection = { + id: 'BRCID10', url: '/orgs/MS/collections/BRCID10/', + canonical_url: 'https://x/ValueSet/BRCID10', version: 'HEAD', name: 'BRCID10', + compose: { include: [] } + }; + const post = jest.fn(async () => ({ + data: [collectionResolve('/orgs/MS/collections/BRCID10/', 'https://x/ValueSet/BRCID10')] + })); + const get = jest.fn(async (url) => { + if (url === '/orgs/MS/collections/BRCID10/') return { data: collection }; + return { data: [] }; + }); + const { provider, httpClient } = makeProvider({ post, get }); + + const vs = await provider.fetchValueSet('https://x/ValueSet/BRCID10', null); + + expect(post).toHaveBeenCalledTimes(1); + expect(vs).toBeTruthy(); + expect(vs.url).toBe('https://x/ValueSet/BRCID10'); + // The authoritative resolve means no /collections/ text search was issued. + const searched = httpClient.get.mock.calls.some(c => /\/collections\/$/.test(c[0])); + expect(searched).toBe(false); + }); + + it('falls back to the per-org search when the resolver is disabled (no token)', async () => { + const provider = new OCLValueSetProvider({ org: 'MS' }); // no token + const get = jest.fn(async (url) => { + if (/\/collections\/$/.test(url)) { + return { + data: [{ + id: 'BRCID10', url: '/orgs/MS/collections/BRCID10/', + canonical_url: 'https://x/ValueSet/BRCID10', version: 'HEAD', name: 'BRCID10', + compose: { include: [] } + }] + }; + } + if (url === '/orgs/') return { data: [{ id: 'MS' }] }; + return { data: [] }; + }); + const post = jest.fn(async () => ({ data: [] })); + provider.httpClient = { get, post }; + + const vs = await provider.fetchValueSet('https://x/ValueSet/BRCID10', null); + + expect(post).not.toHaveBeenCalled(); + expect(vs).toBeTruthy(); + // The text search path did run. + expect(get.mock.calls.some(c => /\/collections\/$/.test(c[0]))).toBe(true); + }); + + it('ignores a resolved repo that is not a collection', async () => { + // OCL resolves the canonical to a *source*, not a collection: not a ValueSet. + const post = jest.fn(async () => ({ + data: [collectionResolve('/orgs/MS/sources/NotACollection/', 'https://x/ValueSet/BRCID10')] + })); + const get = jest.fn(async (url) => { + if (/\/collections\/$/.test(url)) return { data: [] }; // search finds nothing either + if (url === '/orgs/') return { data: [{ id: 'MS' }] }; + return { data: [] }; + }); + const { provider } = makeProvider({ post, get }); + + const vs = await provider.fetchValueSet('https://x/ValueSet/BRCID10', null); + + // Resolver answered a source, so we fell through to the search, which found nothing. + expect(vs).toBeNull(); + }); +}); diff --git a/tx/ocl/vs-ocl.cjs b/tx/ocl/vs-ocl.cjs index 46fd1c6e..617c93d0 100644 --- a/tx/ocl/vs-ocl.cjs +++ b/tx/ocl/vs-ocl.cjs @@ -9,6 +9,7 @@ const { TxParameters } = require('../params'); const { OCLSourceCodeSystemFactory, OCLBackgroundJobQueue } = require('./cs-ocl'); const { PAGE_SIZE, CONCEPT_PAGE_SIZE, FILTERED_CONCEPT_PAGE_SIZE, COLD_CACHE_FRESHNESS_MS } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); +const { OclReferenceResolver } = require('./resolve/reference-resolver'); const { CACHE_VS_DIR, getCacheFilePath } = require('./cache/cache-paths'); const { ensureCacheDirectories, getColdCacheAgeMs, formatCacheAgeMinutes } = require('./cache/cache-utils'); const { computeValueSetExpansionFingerprint } = require('./fingerprint/fingerprint'); @@ -44,6 +45,13 @@ class OCLValueSetProvider extends AbstractValueSetProvider { this.baseUrl = http.baseUrl; this.httpClient = http.client; + // Resolves a canonical to its OCL repo authoritatively. Disabled without a + // token, in which case the search paths below run exactly as before. + this.referenceResolver = new OclReferenceResolver({ + httpClient: this.httpClient, + token: options.token || null + }); + this.valueSetMap = new Map(); this._idMap = new Map(); this.collectionMeta = new Map(); @@ -726,6 +734,11 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } const sourceKeys = await this.#fetchCollectionSourceKeys(meta.conceptsUrl, meta.owner || null); + // Resolve every source's canonical in a single $resolveReference batch and seed + // the per-source cache, replacing one sequential GET per source. The loop below + // then reads from cache; anything the batch could not resolve falls through to + // the individual GET in #getSourceCanonicalUrl, so behaviour is unchanged. + await this.#primeSourceCanonicalsBatch(sourceKeys); for (const { owner, source } of sourceKeys) { const systemUrl = normalizeCanonicalSystem(await this.#getSourceCanonicalUrl(owner, source)); if (systemUrl && !seen.has(systemUrl)) { @@ -1185,6 +1198,15 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } const pending = (async () => { + // Ask OCL which repo holds this canonical. An authoritative answer skips + // the per-org text search below; without one (no token, or the endpoint is + // unavailable) we fall through to it. + const resolvedCollection = await this.#resolveCollectionViaReference(canonicalUrl, version); + if (resolvedCollection) { + this.collectionByCanonicalCache.set(lookupKey, resolvedCollection); + return resolvedCollection; + } + const organizations = await this.#fetchOrganizationIds(); const endpoints = organizations.length > 0 ? organizations.map(orgId => `/orgs/${encodeURIComponent(orgId)}/collections/`) @@ -1251,6 +1273,56 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } } + /** + * Resolve a ValueSet canonical to its OCL collection via $resolveReference, + * then fetch the collection object so it matches the shape the text-search path + * returns. Returns null when the resolver is disabled (no token), the endpoint + * is unavailable, OCL cannot resolve it, or the resolved repo is not a + * collection -- in every case the caller falls back to the search. + */ + async #resolveCollectionViaReference(canonicalUrl, version) { + let resolved; + try { + resolved = await this.referenceResolver.resolve( + version ? { url: canonicalUrl, version } : canonicalUrl + ); + } catch (error) { + console.warn(`[OCL-ValueSet] $resolveReference lookup failed for ${canonicalUrl}: ${error.message}`); + return null; + } + + if (!resolved?.resolved || !resolved.repoUrl) { + return null; + } + + // $resolveReference resolves any repo type; only collections are ValueSets. + if (!/[/]collections[/]/.test(resolved.repoUrl)) { + return null; + } + + let collection; + try { + const response = await this.httpClient.get(resolved.repoUrl); + collection = response?.data || null; + } catch (error) { + console.warn(`[OCL-ValueSet] failed to fetch resolved collection ${resolved.repoUrl}: ${error.message}`); + return null; + } + + if (!collection || typeof collection !== 'object' || !collection.id) { + return null; + } + + // Honour an explicit version: the resolved repo is HEAD unless OCL matched a + // version, so a mismatch means fall back to the version-aware search. + if (version && (collection.version || null) !== version) { + return null; + } + + console.log(`[OCL-ValueSet] $resolveReference resolved ${canonicalUrl} -> ${resolved.repoUrl}`); + return collection; + } + #valueSetMetadataSignature(vs) { const meta = this.#getCollectionMeta(vs); const payload = { @@ -1783,6 +1855,63 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } } + /** + * Resolve many sources' canonicals in one $resolveReference batch, seeding + * sourceCanonicalCache. A no-op when the resolver is disabled (no token) or the + * batch fails: #getSourceCanonicalUrl then falls back to its per-source GET, so + * this only ever saves round trips, never changes the result. + */ + async #primeSourceCanonicalsBatch(sourceKeys) { + if (!this.referenceResolver.isEnabled()) { + return; + } + + // Only sources not already cached, deduplicated by owner|source. + const pending = []; + const seen = new Set(); + for (const { owner, source } of sourceKeys || []) { + if (!owner || !source) { + continue; + } + const key = `${owner}|${source}`; + if (this.sourceCanonicalCache.has(key) || seen.has(key)) { + continue; + } + seen.add(key); + pending.push({ key, ref: `/orgs/${encodeURIComponent(owner)}/sources/${encodeURIComponent(source)}/` }); + } + + if (pending.length === 0) { + return; + } + + let results; + try { + results = await this.referenceResolver.resolveReferences(pending.map(p => p.ref)); + } catch (error) { + console.warn(`[OCL-ValueSet] $resolveReference batch failed: ${error.message}`); + return; + } + + // null => resolver disabled or unavailable mid-flight; leave the cache untouched + // so the per-source GET fallback runs. + if (!Array.isArray(results)) { + return; + } + + let resolvedCount = 0; + for (let i = 0; i < pending.length; i++) { + const canonical = results[i]?.resolved ? results[i].canonical : null; + if (canonical) { + this.sourceCanonicalCache.set(pending[i].key, canonical); + resolvedCount++; + } + } + if (resolvedCount > 0) { + console.log(`[OCL-ValueSet] $resolveReference batch resolved ${resolvedCount}/${pending.length} source canonical(s) in one request`); + } + } + #matches(json, params) { for (const [name, value] of Object.entries(params)) { if (!value) { From 165785de4e20dae859efb9492e3b6a84e2f43b1f Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 16:16:51 -0300 Subject: [PATCH 04/13] feat(ocl): enforce org-only visibility policy Policy decision: an artifact is expected to live in an organization to be visible through the terminology service. User-owned repos (/users/{user}/...) are experimental by convention and are excluded from resolution -- a canonical that $resolveReference resolves to a user-owned repo is treated as unresolved (logged, cached, callers fall back to their search paths). isOclRepoPath therefore accepts only /orgs/ paths, and isOrgOwned checks the explicit owner_type when a payload carries one (falling back to the path shape). This supersedes the earlier reading that dropping /users/ repos in ConceptMap lookups was a bug: it was the intended visibility rule, now stated and tested rather than incidental. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cm-provider.test.js | 23 +++++++++-- tests/ocl/ocl-reference-resolver.test.js | 51 ++++++++++++++++++------ tx/ocl/resolve/reference-resolver.js | 39 +++++++++++++++--- 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/tests/ocl/ocl-cm-provider.test.js b/tests/ocl/ocl-cm-provider.test.js index c7b0006b..4196a3ca 100644 --- a/tests/ocl/ocl-cm-provider.test.js +++ b/tests/ocl/ocl-cm-provider.test.js @@ -460,7 +460,7 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { it('uses the resolved repo and skips the heuristic source search', async () => { const { provider, httpClient } = makeProvider({ - post: resolveReferenceReply('/users/joe/sources/S/') + post: resolveReferenceReply('/orgs/OtherOrg/sources/S/') }); await provider.searchConceptMaps(searchParams); @@ -468,8 +468,25 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { expect(httpClient.post).toHaveBeenCalledTimes(1); // The authoritative answer makes the q= text search unnecessary. expect(getPaths(httpClient)).not.toContain(SEARCH_ENDPOINT); - // ...and a user-owned repo survives, which an /orgs/-only filter would drop. - expect(getPaths(httpClient)).toContain('/users/joe/sources/S/concepts/'); + expect(getPaths(httpClient)).toContain('/orgs/OtherOrg/sources/S/concepts/'); + }); + + it('falls back to the source search when the canonical resolves to a user-owned repo', async () => { + // Org-only policy: user artifacts are experimental and not visible through + // the terminology service, so the resolver reports them as unresolved. + const post = jest.fn(async () => ({ + data: [{ + reference_type: 'canonical', + resolved: true, + result: { url: '/users/joe/sources/S/', owner_type: 'User', canonical_url: 'http://x.org/cs' } + }] + })); + const { provider, httpClient } = makeProvider({ post }); + + await provider.searchConceptMaps(searchParams); + + expect(getPaths(httpClient)).not.toContain('/users/joe/sources/S/concepts/'); + expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); }); it('falls back to the source search when no token is configured', async () => { diff --git a/tests/ocl/ocl-reference-resolver.test.js b/tests/ocl/ocl-reference-resolver.test.js index 0640bf71..62688d0d 100644 --- a/tests/ocl/ocl-reference-resolver.test.js +++ b/tests/ocl/ocl-reference-resolver.test.js @@ -2,6 +2,7 @@ const { OclReferenceResolver, normalizeReference, isOclRepoPath, + isOrgOwned, RESOLVE_PATH } = require('../../tx/ocl/resolve/reference-resolver'); @@ -77,19 +78,20 @@ function httpError(status, data) { return error; } -describe('isOclRepoPath', () => { +describe('isOclRepoPath (org-only policy)', () => { it.each([ ['/orgs/CIEL/sources/CIEL/'], ['/orgs/CIEL/sources/CIEL/HEAD/'], ['/orgs/CIEL/collections/C/'], - // The point: user-owned repos are real and must not be filtered out. - ['/users/joe/sources/S/'], - [' /users/joe/sources/S/ '] + [' /orgs/CIEL/sources/CIEL/ '] ])('accepts %s', input => { expect(isOclRepoPath(input)).toBe(true); }); it.each([ + // User-owned artifacts are experimental by convention: an artifact is + // expected to live in an org to be visible through the terminology service. + ['/users/joe/sources/S/'], ['http://loinc.org'], ['/sources/S/'], ['/orgs/'], @@ -103,6 +105,23 @@ describe('isOclRepoPath', () => { }); }); +describe('isOrgOwned', () => { + it('prefers an explicit owner_type', () => { + expect(isOrgOwned({ owner_type: 'Organization', url: '/users/joe/sources/S/' })).toBe(true); + expect(isOrgOwned({ owner_type: 'User', url: '/orgs/A/sources/S/' })).toBe(false); + expect(isOrgOwned({ ownerType: 'Organization' })).toBe(true); + }); + + it('falls back to the path shape when owner_type is absent', () => { + expect(isOrgOwned({ url: '/orgs/A/sources/S/' })).toBe(true); + expect(isOrgOwned({ url: '/users/joe/sources/S/' })).toBe(false); + }); + + it.each([[null], [undefined], ['x'], [{}]])('rejects %p', input => { + expect(isOrgOwned(input)).toBe(false); + }); +}); + describe('normalizeReference', () => { it('passes a relative path string through as a string', () => { expect(normalizeReference('/orgs/CIEL/sources/CIEL/')).toBe('/orgs/CIEL/sources/CIEL/'); @@ -251,16 +270,24 @@ describe('OclReferenceResolver resolution', () => { expect(r.canonical).toBe('https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS'); }); - it('resolves a user-owned repo (an /orgs/-only filter would drop these)', async () => { + it('treats a canonical that resolves to a user-owned repo as unresolved (org-only policy)', async () => { const httpClient = { - post: jest.fn(async () => ({ data: [oclEntry({ url: '/users/joe/sources/S/' })] })) + post: jest.fn(async () => ({ + data: [oclEntry({ url: '/users/joe/sources/S/', ownerType: 'User', owner: 'joe' })] + })) }; - const resolver = makeResolver({ httpClient }); + const logger = silentLogger(); + const resolver = makeResolver({ httpClient, logger }); - const result = await resolver.resolve('/users/joe/sources/S/'); + const result = await resolver.resolve('http://joe.example.org/cs'); - expect(result.repoUrl).toBe('/users/joe/sources/S/'); - expect(isOclRepoPath(result.repoUrl)).toBe(true); + expect(result.resolved).toBe(false); + expect(result.repoUrl).toBeNull(); + expect(logger.info).toHaveBeenCalledWith(expect.stringMatching(/user-owned repo.*org-only policy/)); + + // Policy outcome is deterministic: cached, no second round trip. + await resolver.resolve('http://joe.example.org/cs'); + expect(httpClient.post).toHaveBeenCalledTimes(1); }); it('returns an empty array and issues no request for no references', async () => { @@ -312,7 +339,7 @@ describe('OclReferenceResolver resolution', () => { post: jest.fn(async () => ({ data: [{ resolved: true, - result: { url: '/orgs/A/sources/S/', canonicalUrl: 'http://a.org/cs', ownerType: 'User' } + result: { url: '/orgs/A/sources/S/', canonicalUrl: 'http://a.org/cs', ownerType: 'Organization' } }] })) }; @@ -321,7 +348,7 @@ describe('OclReferenceResolver resolution', () => { const r = await resolver.resolve('/orgs/A/sources/S/'); expect(r.canonical).toBe('http://a.org/cs'); - expect(r.ownerType).toBe('User'); + expect(r.ownerType).toBe('Organization'); }); it("prefers OCL's own request echo over the submitted body", async () => { diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js index 4d2372a9..f4f91001 100644 --- a/tx/ocl/resolve/reference-resolver.js +++ b/tx/ocl/resolve/reference-resolver.js @@ -14,9 +14,10 @@ const RESOLVE_PATH = '/$resolveReference/'; -// OCL repos are owned by an org OR a user. Matching only /orgs/ silently drops -// every user-owned repo. -const REPO_PATH_PATTERN = /^\/(orgs|users)\/[^/]+\//; +// Org-only visibility policy: an artifact is expected to live in an organization +// to be visible through the terminology service. User-owned repos (/users/...) +// are experimental by convention and are excluded from discovery AND resolution. +const REPO_PATH_PATTERN = /^\/orgs\/[^/]+\//; // Reference object fields forwarded to OCL. `namespace` is intentionally absent. const BODY_FIELDS = [ @@ -32,13 +33,29 @@ const BODY_FIELDS = [ ]; /** - * True for a relative OCL repo path under either owner type, e.g. - * `/orgs/CIEL/sources/CIEL/` or `/users/joe/sources/S/`. + * True for a relative OCL repo path the terminology service may serve — i.e. an + * organization-owned one (`/orgs/CIEL/sources/CIEL/`). User-owned paths + * (`/users/joe/...`) are rejected by policy. */ function isOclRepoPath(value) { return REPO_PATH_PATTERN.test(String(value == null ? '' : value).trim()); } +/** + * Org-only policy check for an OCL repo payload or resolve result. Prefers the + * explicit owner_type when present; falls back to the path shape. + */ +function isOrgOwned(repo) { + if (!repo || typeof repo !== 'object') { + return false; + } + const ownerType = repo.owner_type || repo.ownerType || null; + if (ownerType) { + return ownerType === 'Organization'; + } + return isOclRepoPath(repo.url); +} + /** * Accepts either a relative/canonical URL string or an expanded reference object, * returning the request-body form OCL expects. @@ -224,7 +241,16 @@ class OclReferenceResolver { } misses.forEach(({ body, index }, position) => { - const value = normalizeResult(payload[position], body); + let value = normalizeResult(payload[position], body); + // Org-only policy: a canonical resolving to a user-owned repo is treated as + // unresolved — user artifacts are experimental and not visible through the + // terminology service. Cached: the policy outcome is deterministic. + if (value.resolved && !isOrgOwned({ owner_type: value.ownerType, url: value.repoUrl })) { + this.#logger.info( + `[OCL] $resolveReference resolved ${cacheKey(body)} to a user-owned repo (${value.repoUrl}); org-only policy treats it as unresolved` + ); + value = unresolved(body); + } this.#cache.set(cacheKey(body), value); output[index] = value; }); @@ -271,5 +297,6 @@ module.exports = { OclReferenceResolver, normalizeReference, isOclRepoPath, + isOrgOwned, RESOLVE_PATH }; From 6a90e6a9fc41499cc607379e20638556ce9bfa29 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 16:16:52 -0300 Subject: [PATCH 05/13] perf(ocl): discover sources and collections via the global listings Discovery enumerated /orgs/ and then listed /orgs/{org}/sources/ (and /collections/) for every org -- N+1 listing requests, repeated on every refresh cycle. The global /sources/ and /collections/ listings return the same set in one paginated crawl; verified live: 15 orgs, per-org discovery fetched 417 sources, the global listing reports num_found=417 and boot now logs the same "Fetched 417 sources" through a single crawl. Entries are filtered with isOrgOwned() to honour the org-only visibility policy (the global listing includes user-owned repos, which per-org enumeration never saw -- live boot showed 463 collections globally, 450 kept after the filter). The per-org path remains as fallback for instances where the global listing is unavailable or empty. Enumeration itself stays: a terminology server's catalog is built by discovery, not by request traffic. This changes how the catalog is listed, not whether. Co-Authored-By: Claude Opus 4.8 --- tx/ocl/README.md | 31 +++++++++++++++++++++++++------ tx/ocl/cs-ocl.cjs | 31 +++++++++++++++++++++++++++++-- tx/ocl/vs-ocl.cjs | 32 +++++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/tx/ocl/README.md b/tx/ocl/README.md index c46324c2..2ab7c5db 100644 --- a/tx/ocl/README.md +++ b/tx/ocl/README.md @@ -39,14 +39,33 @@ In FHIRsmith terms, these providers are loaded by `tx/library.js` when a source ## Runtime flow ### Metadata discovery CodeSystems (`cs-ocl.js`): -- discover orgs via `/orgs/` -- for each org, discover sources via `/orgs/{org}/sources/` -- fallback to `/sources/` if org listing is unavailable +- prefer one paginated crawl of the global `/sources/` listing (filtered to + organization-owned entries) +- fallback: discover orgs via `/orgs/`, then `/orgs/{org}/sources/` per org ValueSets (`vs-ocl.js`): -- discover orgs via `/orgs/` -- discover collections via `/orgs/{org}/collections/` -- fallback to `/collections/` +- prefer one paginated crawl of the global `/collections/` listing (filtered to + organization-owned entries) +- fallback: discover orgs via `/orgs/`, then `/orgs/{org}/collections/` per org + +### Visibility policy (org-only) +An artifact is expected to live in an **organization** to be visible through the +terminology service. User-owned repos (`/users/{user}/...`) are experimental by +convention and are excluded from both discovery and `$resolveReference` results +(a canonical resolving to a user-owned repo is treated as unresolved). + +### Canonical resolution via `$resolveReference` +With a `token=` configured on the `ocl:` source line, the providers resolve +"which repo holds this canonical URL?" through OCL's +[`$resolveReference`](https://docs.openconceptlab.org/en/latest/oclapi/apireference/resolveReference.html) +(global namespace) instead of iterating listings and matching `canonical_url` +client-side. Used for ConceptMap source-system search, ValueSet lookup by +canonical, and batched resolution of a collection's compose source canonicals. + +Without a token nothing changes: `$resolveReference` is authenticated on every +instance probed (while the listing endpoints are public), so the resolver is +constructed disabled and every caller keeps its previous search path. It also +disables itself for the process on `404`/`401`/`403`. ConceptMaps (`cm-ocl.js`): - fetch by id via `/mappings/{id}/` diff --git a/tx/ocl/cs-ocl.cjs b/tx/ocl/cs-ocl.cjs index 89d06e2e..66d411fc 100644 --- a/tx/ocl/cs-ocl.cjs +++ b/tx/ocl/cs-ocl.cjs @@ -7,6 +7,7 @@ const { SearchFilterText } = require('../library/designations'); const { PAGE_SIZE, CONCEPT_PAGE_SIZE, COLD_CACHE_FRESHNESS_MS, OCL_CODESYSTEM_MARKER_EXTENSION } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); const { fetchAllPages, extractItemsAndNext } = require('./http/pagination'); +const { isOrgOwned } = require('./resolve/reference-resolver'); const { CACHE_CS_DIR, CACHE_VS_DIR, getCacheFilePath } = require('./cache/cache-paths'); const { ensureCacheDirectories, getColdCacheAgeMs, formatCacheAgeMinutes } = require('./cache/cache-utils'); const { computeCodeSystemFingerprint } = require('./fingerprint/fingerprint'); @@ -192,10 +193,36 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { } async #fetchSourcesForDiscovery() { + // Prefer the global listing: one paginated crawl instead of one listing per + // org (N+1 requests). Org-only policy: user-owned sources are experimental by + // convention and not visible to the terminology service, so filter them out. + try { + const all = await this.#fetchAllPages('/sources/'); + if (Array.isArray(all) && all.length > 0) { + const seen = new Set(); + const orgOwned = []; + for (const source of all) { + if (!source || typeof source !== 'object' || !isOrgOwned(source)) { + continue; + } + const key = this.#sourceIdentity(source); + if (seen.has(key)) { + continue; + } + seen.add(key); + orgOwned.push(source); + } + if (orgOwned.length > 0) { + return orgOwned; + } + } + } catch (error) { + console.warn(`[OCL] Global /sources/ listing failed (${error.message}); falling back to per-org discovery`); + } + const organizations = await this.#fetchOrganizationIds(); if (organizations.length === 0) { - // Fallback for OCL instances that expose global listing but not org listing. - return await this.#fetchAllPages('/sources/'); + return []; } const allSources = []; diff --git a/tx/ocl/vs-ocl.cjs b/tx/ocl/vs-ocl.cjs index 617c93d0..fed9244c 100644 --- a/tx/ocl/vs-ocl.cjs +++ b/tx/ocl/vs-ocl.cjs @@ -9,7 +9,7 @@ const { TxParameters } = require('../params'); const { OCLSourceCodeSystemFactory, OCLBackgroundJobQueue } = require('./cs-ocl'); const { PAGE_SIZE, CONCEPT_PAGE_SIZE, FILTERED_CONCEPT_PAGE_SIZE, COLD_CACHE_FRESHNESS_MS } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); -const { OclReferenceResolver } = require('./resolve/reference-resolver'); +const { OclReferenceResolver, isOrgOwned } = require('./resolve/reference-resolver'); const { CACHE_VS_DIR, getCacheFilePath } = require('./cache/cache-paths'); const { ensureCacheDirectories, getColdCacheAgeMs, formatCacheAgeMinutes } = require('./cache/cache-utils'); const { computeValueSetExpansionFingerprint } = require('./fingerprint/fingerprint'); @@ -1383,10 +1383,36 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } async #fetchCollectionsForDiscovery() { + // Prefer the global listing: one paginated crawl instead of one listing per + // org (N+1 requests). Org-only policy: user-owned collections are + // experimental by convention and not visible to the terminology service. + try { + const all = await this.#fetchAllPages('/collections/'); + if (Array.isArray(all) && all.length > 0) { + const seen = new Set(); + const orgOwned = []; + for (const collection of all) { + if (!collection || typeof collection !== 'object' || !isOrgOwned(collection)) { + continue; + } + const key = this.#collectionIdentity(collection); + if (seen.has(key)) { + continue; + } + seen.add(key); + orgOwned.push(collection); + } + if (orgOwned.length > 0) { + return orgOwned; + } + } + } catch (error) { + console.warn(`[OCL-ValueSet] Global /collections/ listing failed (${error.message}); falling back to per-org discovery`); + } + const organizations = await this.#fetchOrganizationIds(); if (organizations.length === 0) { - // Fallback for OCL instances that expose global listing but not org listing. - return await this.#fetchAllPages('/collections/'); + return []; } const allCollections = []; From 0549620e42a4093431665598131e141f0e5696ed Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 16:31:47 -0300 Subject: [PATCH 06/13] fix(ocl): fetch source mappings directly instead of walking every concept searchConceptMaps has no concept code -- it asks "what mappings does this source have?" -- but answered that with the per-concept endpoint: list the concepts, then issue one request per concept and union the results. Two problems, measured against the live OCL instance: - Correctness. The concept listing is capped at maxSearchPages (10 x 100 = 1000). LOINC has 184,683 concepts, so it only ever saw 0.5% of them, and any mapping on a concept past the first 1000 was silently invisible. No error -- just fewer results. - Cost. Up to 1000 sequential requests per source. ConceptMap searches on loinc and snomed timed out (45s+) and returned nothing at all; in production the same shows up as 504s. {source}/mappings/ answers the actual question in one paginated call. Verified equivalent before switching: for a source with 2 mappings both paths return the identical set, one in 1 request instead of 4. After the change, live: http://loinc.org timeout(45s+), 0 results -> 200, 1 ConceptMap http://snomed.info/sct timeout(45s+), 0 results -> 200, 5 ConceptMaps The per-concept endpoint stays where it belongs: findConceptMapForTranslation has a sourceCode and asks about that one concept, so its single targeted request is already the right call. Untouched. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cm-provider.test.js | 9 +++++++-- tx/ocl/cm-ocl.cjs | 28 +++++++++------------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/tests/ocl/ocl-cm-provider.test.js b/tests/ocl/ocl-cm-provider.test.js index 4196a3ca..a67310a4 100644 --- a/tests/ocl/ocl-cm-provider.test.js +++ b/tests/ocl/ocl-cm-provider.test.js @@ -214,6 +214,11 @@ describe('OCLConceptMapProvider', () => { ]; const getMock = jest.fn().mockImplementation((url) => { + // {source}/mappings/ — one request returns every mapping the source owns. + // Must precede the '/sources/' branch below, which would otherwise swallow it. + if (url.endsWith('/mappings/') && !url.includes('/concepts/')) { + return Promise.resolve({ data: mappings }); + } // source search — resolve canonical for SourceA if (url.includes('/sources/') && !url.includes('/concepts/')) { return Promise.resolve({ @@ -468,7 +473,7 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { expect(httpClient.post).toHaveBeenCalledTimes(1); // The authoritative answer makes the q= text search unnecessary. expect(getPaths(httpClient)).not.toContain(SEARCH_ENDPOINT); - expect(getPaths(httpClient)).toContain('/orgs/OtherOrg/sources/S/concepts/'); + expect(getPaths(httpClient)).toContain('/orgs/OtherOrg/sources/S/mappings/'); }); it('falls back to the source search when the canonical resolves to a user-owned repo', async () => { @@ -485,7 +490,7 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { await provider.searchConceptMaps(searchParams); - expect(getPaths(httpClient)).not.toContain('/users/joe/sources/S/concepts/'); + expect(getPaths(httpClient)).not.toContain('/users/joe/sources/S/mappings/'); expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); }); diff --git a/tx/ocl/cm-ocl.cjs b/tx/ocl/cm-ocl.cjs index f5cf3f3f..6d4a7af6 100644 --- a/tx/ocl/cm-ocl.cjs +++ b/tx/ocl/cm-ocl.cjs @@ -153,30 +153,20 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { const allMappings = []; for (const sourcePath of sourcePaths) { const normalizedPath = this.#normalizeSourcePath(sourcePath); - let concepts; try { - concepts = await this.#fetchAllPages( - `${normalizedPath}concepts/`, { limit: PAGE_SIZE }, this.maxSearchPages + // Ask the source for its mappings directly. Walking concepts and asking + // each one costs a request per concept and, worse, only ever sees the + // first maxSearchPages of them -- for LOINC that is 1000 of 184683 + // concepts (0.5%), so any mapping past that point was silently invisible. + // Verified live: both paths return the identical mapping set. + const mappings = await this.#fetchAllPages( + `${normalizedPath}mappings/`, { limit: PAGE_SIZE }, this.maxSearchPages ); + allMappings.push(...mappings); } catch (_err) { + // source exposes no mappings endpoint or is inaccessible — skip continue; } - - for (const concept of concepts) { - const code = concept.id || concept.mnemonic; - if (!code) { - continue; - } - try { - const mappings = await this.#fetchAllPages( - `${normalizedPath}concepts/${encodeURIComponent(code)}/mappings/`, - { limit: PAGE_SIZE }, 2 - ); - allMappings.push(...mappings); - } catch (_err) { - // concept has no mappings or endpoint inaccessible — skip - } - } } const sourceUrlsToResolve = new Set(); From e35f6f11fe565118b73beb52d53c12533bf04e8d Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 16:58:33 -0300 Subject: [PATCH 07/13] feat(ocl): batch mapping source canonicals via $resolveReference After collecting a source's mappings, #ensureCanonicalForSourceUrls translated each from/to_source_url into its canonical with one sequential GET per source repo. $resolveReference answers the same question -- the result carries the repo's canonical_url -- for the whole set in a single POST. The per-source GET loop stays as the fallback for whatever the batch could not resolve (no token, endpoint unavailable, individual misses), so behaviour without a token is unchanged. This was the last canonical<->repo translation in tx/ocl still done by per-item requests; with it, every such lookup goes through $resolveReference when a token is configured: canonical -> repo ConceptMap source-system, ValueSet by canonical repo -> canonical ValueSet compose sources (batch), mapping sources (batch) Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cm-provider.test.js | 51 ++++++++++++++++++++++++++ tx/ocl/cm-ocl.cjs | 59 ++++++++++++++++++++++++++----- 2 files changed, 101 insertions(+), 9 deletions(-) diff --git a/tests/ocl/ocl-cm-provider.test.js b/tests/ocl/ocl-cm-provider.test.js index a67310a4..04af708d 100644 --- a/tests/ocl/ocl-cm-provider.test.js +++ b/tests/ocl/ocl-cm-provider.test.js @@ -525,6 +525,57 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { expect(getPaths(httpClient)).toContain(SEARCH_ENDPOINT); }); + it('resolves mapping source canonicals in one batch, not one GET per source', async () => { + // Flow: resolve source-system (1 ref) -> fetch {source}/mappings/ -> resolve + // the from/to source canonicals of the mappings in a single batched POST. + const post = jest.fn(async (path, body) => ({ + data: body.map(ref => { + const url = typeof ref === 'string' ? ref : ref.url; + const repo = url.startsWith('/') ? url : '/orgs/TestOrg/sources/A/'; + return { + reference_type: url.startsWith('/') ? 'relative' : 'canonical', + resolved: true, + result: { + url: repo, + owner_type: 'Organization', + type: 'Source', + canonical_url: `http://canon.example.org${repo}` + } + }; + }) + })); + const get = jest.fn(async (url) => { + if (url.endsWith('/mappings/')) { + return { + data: [makeMapping({ + from_source_url: '/orgs/TestOrg/sources/SourceA/', + to_source_url: '/orgs/TestOrg/sources/SourceB/' + })] + }; + } + return { data: [] }; + }); + const { provider, httpClient } = makeProvider({ post }); + httpClient.get = get; + provider.httpClient.get = get; + + const results = await provider.searchConceptMaps(searchParams); + + // POST #1 resolves the source-system; POST #2 is the batch with BOTH + // mapping source paths in one request. + expect(post).toHaveBeenCalledTimes(2); + expect(post.mock.calls[1][1]).toEqual([ + '/orgs/TestOrg/sources/SourceA/', + '/orgs/TestOrg/sources/SourceB/' + ]); + // No per-source detail GETs: only the mappings listing was fetched. + const detailGets = get.mock.calls.filter(c => /\/sources\/Source[AB]\/$/.test(c[0])); + expect(detailGets).toHaveLength(0); + // Aggregation used the canonicals from the batch. + expect(results).toHaveLength(1); + expect(results[0].jsonObj.group[0].source).toBe('http://canon.example.org/orgs/TestOrg/sources/SourceA/'); + }); + // Regression: searchConceptMaps used to lower-case every param value. The old // text search tolerated it (#norm lower-cases anyway), but $resolveReference // matches the canonical exactly, so a lower-cased URL never resolved. diff --git a/tx/ocl/cm-ocl.cjs b/tx/ocl/cm-ocl.cjs index 6d4a7af6..43a175ff 100644 --- a/tx/ocl/cm-ocl.cjs +++ b/tx/ocl/cm-ocl.cjs @@ -625,17 +625,52 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { } async #ensureCanonicalForSourceUrls(sourceUrls) { + // Deduplicate the repo paths that still need a canonical. + const pending = []; + const seen = new Set(); for (const sourceUrl of sourceUrls || []) { const sourceKey = this.#norm(sourceUrl); - if (!sourceKey || this._canonicalBySourceUrl.has(sourceKey)) { + if (!sourceKey || this._canonicalBySourceUrl.has(sourceKey) || seen.has(sourceKey)) { continue; } - const sourcePath = String(sourceUrl || '').trim(); if (!isOclRepoPath(sourcePath)) { continue; } + seen.add(sourceKey); + pending.push(sourcePath); + } + + if (pending.length === 0) { + return; + } + + // One $resolveReference batch instead of one GET per source: the result + // carries the repo's canonical_url. Anything the batch cannot resolve falls + // through to the per-source GETs below, so behaviour without a token (or on + // failure) is unchanged. + if (this.referenceResolver.isEnabled()) { + try { + const results = await this.referenceResolver.resolveReferences(pending); + if (Array.isArray(results)) { + for (let i = 0; i < pending.length; i++) { + const r = results[i]; + if (!r?.resolved || !r.canonical) { + continue; + } + const resolvedSourceUrl = r.repoUrl || pending[i]; + this.#recordCanonicalForSource(pending[i], resolvedSourceUrl, r.canonical); + } + } + } catch (error) { + console.warn(`[OCL] $resolveReference batch for source canonicals failed: ${error.message}`); + } + } + for (const sourcePath of pending) { + if (this._canonicalBySourceUrl.has(this.#norm(sourcePath))) { + continue; + } try { const response = await this.httpClient.get(sourcePath); const source = response.data || {}; @@ -644,13 +679,7 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { if (!canonical) { continue; } - - const canonicalKey = this.#norm(canonical); - if (!this._sourceUrlsByCanonical.has(canonicalKey)) { - this._sourceUrlsByCanonical.set(canonicalKey, new Set()); - } - this._sourceUrlsByCanonical.get(canonicalKey).add(resolvedSourceUrl); - this._canonicalBySourceUrl.set(this.#norm(resolvedSourceUrl), canonical); + this.#recordCanonicalForSource(sourcePath, resolvedSourceUrl, canonical); } catch (e) { // Ignore source lookup failures and continue resolving remaining sources. continue; @@ -658,6 +687,18 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { } } + #recordCanonicalForSource(requestedPath, resolvedSourceUrl, canonical) { + const canonicalKey = this.#norm(canonical); + if (!this._sourceUrlsByCanonical.has(canonicalKey)) { + this._sourceUrlsByCanonical.set(canonicalKey, new Set()); + } + this._sourceUrlsByCanonical.get(canonicalKey).add(resolvedSourceUrl); + this._canonicalBySourceUrl.set(this.#norm(resolvedSourceUrl), canonical); + // Also index under the exact path we were asked about, so the "already + // resolved" checks hit even when OCL's repo url differs in shape. + this._canonicalBySourceUrl.set(this.#norm(requestedPath), canonical); + } + #queryTokenFromSystem(systemUrl) { const raw = String(systemUrl || '').trim().replace(/\/+$/, ''); if (!raw) { From 23053e7c00f5187095857be1e2273dcbafa8f56d Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 18:20:43 -0300 Subject: [PATCH 08/13] feat(ocl): serve the released version as the default, with HEAD as |HEAD OCL's own resolution says a source's default version is its latest RELEASE (HEAD only when nothing is released) -- measured live: $resolveReference for http://loinc.org with no version answers 2.82, type "Source Version". But discovery listings only report HEAD, so FHIRsmith registered HEAD-only: versionless requests served the DRAFT, and requests for the released version got "unknown". A/B against main confirmed both pre-existing. With a token, discovery now batch-resolves every canonical and, where the default differs from HEAD, rewrites the snapshot entry to the release -- CodeSystem resource, meta and a version-scoped concepts URL (/{version}/concepts/, verified live) -- keeping the HEAD meta as an extra variant. getSourceMetas() returns defaults first, so registerProvider's first-wins unversioned key makes the release the versionless answer, while |HEAD and |{release} both resolve explicitly. Only new-or-changed canonicals are re-resolved on the minute refresh, so a quiet cycle costs no extra requests; a release being published or deleted flips the entry checksum, surfaces as "changed", and the refresh creates factories for versions that appeared. Without a token, discovery stays HEAD-only exactly as before. Two supporting changes ride along because the feature depends on them: - resolveReferences() now chunks batches at 100 references (the live instance 403s somewhere past 150) and takes a bypassCache option so refresh sees release changes; the cache is refreshed, not invalidated. - OCLSourceCodeSystemFactory registered itself under a SHA-256 of `system|version` while hasExactFactory/#findFactory look up the PLAIN string, so exact-version matching could never succeed -- only the unversioned `system|` alias worked. The key is in-memory only (maps, job keys, logs); it is now the plain string. Found the moment version-aware factory creation needed hasExactFactory to actually work. Live, on cmed (release 20230109): before: no version -> 200 version=HEAD ; version=20230109 -> 422 unknown after: no version -> 200 version=20230109 ; |20230109 -> 200 ; |HEAD -> 200 Boot reports the blast radius on this instance: [OCL] 176 code system(s) defaulting to a released version (HEAD kept as |HEAD) Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cs-default-version.test.js | 125 +++++++++++++++++++++ tests/ocl/ocl-reference-resolver.test.js | 30 +++++ tx/ocl/README.md | 10 ++ tx/ocl/cs-ocl.cjs | 135 +++++++++++++++++++++-- tx/ocl/resolve/reference-resolver.js | 43 ++++++-- 5 files changed, 321 insertions(+), 22 deletions(-) create mode 100644 tests/ocl/ocl-cs-default-version.test.js diff --git a/tests/ocl/ocl-cs-default-version.test.js b/tests/ocl/ocl-cs-default-version.test.js new file mode 100644 index 00000000..e8a9da69 --- /dev/null +++ b/tests/ocl/ocl-cs-default-version.test.js @@ -0,0 +1,125 @@ +// Default-version resolution for OCL CodeSystems: OCL's own resolution says the +// latest RELEASE is a source's default version (HEAD only when nothing is +// released), but discovery listings only ever report HEAD. With a token, the +// provider batch-resolves every canonical via $resolveReference and registers +// BOTH versions — the release as the default (what a versionless request gets) +// and HEAD as an explicit |HEAD variant. + +const { OCLCodeSystemProvider } = require('../../tx/ocl/cs-ocl'); +const { OclReferenceResolver } = require('../../tx/ocl/resolve/reference-resolver'); + +const CANONICAL = 'https://gov.br/anvisa/fhir/CodeSystem/cmed'; + +function cmedSource() { + return { + id: 'cmed', + short_code: 'cmed', + owner: 'ANVISA', + owner_type: 'Organization', + url: '/orgs/ANVISA/sources/cmed/', + canonical_url: CANONICAL, + version: 'HEAD', + concepts_url: '/orgs/ANVISA/sources/cmed/concepts/', + checksums: { standard: 'x' }, + updated_at: '2026-01-01T00:00:00Z' + }; +} + +// $resolveReference reply: default version is the released 20230109. +function releaseReply() { + return { + reference_type: 'canonical', + resolved: true, + result: { + url: '/orgs/ANVISA/sources/cmed/', + owner: 'ANVISA', + owner_type: 'Organization', + version: '20230109', + type: 'Source Version', + canonical_url: CANONICAL + } + }; +} + +function makeProvider({ token = 'Token x', post } = {}) { + const provider = new OCLCodeSystemProvider({ baseUrl: 'https://ocl.example.org', token }); + const httpClient = { + get: jest.fn(async (url) => { + if (url === '/sources/') { + return { data: { results: [cmedSource()], num_found: 1 } }; + } + return { data: [] }; + }), + post: post || jest.fn(async () => ({ data: [releaseReply()] })) + }; + provider.httpClient = httpClient; + provider.referenceResolver = new OclReferenceResolver({ + httpClient, token, logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() } + }); + return { provider, httpClient }; +} + +describe('OCL CodeSystem default-version resolution', () => { + it('registers the release as default and keeps HEAD as an explicit variant', async () => { + const { provider, httpClient } = makeProvider(); + + const listed = await provider.listCodeSystems('5.0', null); + const metas = provider.getSourceMetas(); + + // One batched $resolveReference for the discovered canonicals. + expect(httpClient.post).toHaveBeenCalledTimes(1); + expect(httpClient.post.mock.calls[0][1]).toEqual([CANONICAL]); + + // The listed CodeSystem is the release, not the HEAD draft. + expect(listed).toHaveLength(1); + expect(listed[0].jsonObj.version).toBe('20230109'); + + // Both versions exist; the release comes FIRST so registerProvider's + // first-wins unversioned key makes it the versionless default. + expect(metas.map(m => m.version)).toEqual(['20230109', 'HEAD']); + expect(metas[0].conceptsUrl).toBe('/orgs/ANVISA/sources/cmed/20230109/concepts/'); + expect(metas[1].conceptsUrl).toBe('/orgs/ANVISA/sources/cmed/concepts/'); + expect(metas[0].canonicalUrl).toBe(CANONICAL); + }); + + it('stays HEAD-only without a token (behaviour unchanged)', async () => { + const { provider, httpClient } = makeProvider({ token: null }); + + const listed = await provider.listCodeSystems('5.0', null); + const metas = provider.getSourceMetas(); + + expect(httpClient.post).not.toHaveBeenCalled(); + expect(listed[0].jsonObj.version).toBe('HEAD'); + expect(metas.map(m => m.version)).toEqual(['HEAD']); + }); + + it('stays HEAD-only when HEAD is the default (nothing released)', async () => { + const post = jest.fn(async () => ({ + data: [{ + reference_type: 'canonical', + resolved: true, + result: { url: '/orgs/ANVISA/sources/cmed/', owner_type: 'Organization', version: 'HEAD', type: 'Source', canonical_url: CANONICAL } + }] + })); + const { provider } = makeProvider({ post }); + + const listed = await provider.listCodeSystems('5.0', null); + const metas = provider.getSourceMetas(); + + expect(listed[0].jsonObj.version).toBe('HEAD'); + expect(metas.map(m => m.version)).toEqual(['HEAD']); + }); + + it('keeps discovery working when $resolveReference is unavailable', async () => { + const error = new Error('Request failed with status code 404'); + error.response = { status: 404 }; + const { provider } = makeProvider({ post: jest.fn().mockRejectedValue(error) }); + + const listed = await provider.listCodeSystems('5.0', null); + const metas = provider.getSourceMetas(); + + // Falls back to HEAD-only — never blocks discovery. + expect(listed).toHaveLength(1); + expect(metas.map(m => m.version)).toEqual(['HEAD']); + }); +}); diff --git a/tests/ocl/ocl-reference-resolver.test.js b/tests/ocl/ocl-reference-resolver.test.js index 62688d0d..54f261ce 100644 --- a/tests/ocl/ocl-reference-resolver.test.js +++ b/tests/ocl/ocl-reference-resolver.test.js @@ -425,6 +425,36 @@ describe('OclReferenceResolver batching', () => { }); }); +describe('OclReferenceResolver chunking', () => { + it('splits a large batch into POSTs of at most 100 and preserves order', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + const refs = Array.from({ length: 250 }, (_, i) => `ref${i}`); + + const results = await resolver.resolveReferences(refs); + + expect(httpClient.post).toHaveBeenCalledTimes(3); + expect(httpClient.post.mock.calls.map(c => c[1].length)).toEqual([100, 100, 50]); + expect(results).toHaveLength(250); + expect(results[0].repoUrl).toBe('/orgs/X/sources/ref0/'); + expect(results[249].repoUrl).toBe('/orgs/X/sources/ref249/'); + }); + + it('bypassCache re-asks OCL and refreshes the cache', async () => { + const httpClient = echoClient(); + const resolver = makeResolver({ httpClient }); + + await resolver.resolve('a'); + await resolver.resolveReferences(['a'], { bypassCache: true }); + + expect(httpClient.post).toHaveBeenCalledTimes(2); + + // Cache was refreshed, not invalidated: a third plain call is a cache hit. + await resolver.resolve('a'); + expect(httpClient.post).toHaveBeenCalledTimes(2); + }); +}); + describe('OclReferenceResolver caching', () => { it('serves a repeat reference from cache', async () => { const httpClient = echoClient(); diff --git a/tx/ocl/README.md b/tx/ocl/README.md index 2ab7c5db..ca406dd4 100644 --- a/tx/ocl/README.md +++ b/tx/ocl/README.md @@ -67,6 +67,16 @@ instance probed (while the listing endpoints are public), so the resolver is constructed disabled and every caller keeps its previous search path. It also disables itself for the process on `404`/`401`/`403`. +### CodeSystem default versions (release vs HEAD) +OCL's own resolution treats a source's **latest release** as its default version +(HEAD only when nothing is released), but discovery listings only ever report +HEAD. With a token, discovery batch-resolves every canonical through +`$resolveReference` and registers **both** versions: the release becomes what a +versionless request gets (matching OCL), and HEAD stays reachable via an +explicit `version=HEAD`. Only canonicals that are new or changed are re-resolved +on refresh, so a steady-state cycle costs no extra requests. Without a token, +discovery stays HEAD-only as before. + ConceptMaps (`cm-ocl.js`): - fetch by id via `/mappings/{id}/` - search via `/mappings/` or `/orgs/{org}/mappings/` diff --git a/tx/ocl/cs-ocl.cjs b/tx/ocl/cs-ocl.cjs index 66d411fc..3106d4cc 100644 --- a/tx/ocl/cs-ocl.cjs +++ b/tx/ocl/cs-ocl.cjs @@ -7,7 +7,7 @@ const { SearchFilterText } = require('../library/designations'); const { PAGE_SIZE, CONCEPT_PAGE_SIZE, COLD_CACHE_FRESHNESS_MS, OCL_CODESYSTEM_MARKER_EXTENSION } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); const { fetchAllPages, extractItemsAndNext } = require('./http/pagination'); -const { isOrgOwned } = require('./resolve/reference-resolver'); +const { OclReferenceResolver, isOrgOwned } = require('./resolve/reference-resolver'); const { CACHE_CS_DIR, CACHE_VS_DIR, getCacheFilePath } = require('./cache/cache-paths'); const { ensureCacheDirectories, getColdCacheAgeMs, formatCacheAgeMinutes } = require('./cache/cache-utils'); const { computeCodeSystemFingerprint } = require('./fingerprint/fingerprint'); @@ -44,6 +44,16 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { this.baseUrl = http.baseUrl; this.httpClient = http.client; + // Resolves each source's DEFAULT version (latest release, else HEAD) so both + // get registered. Disabled without a token, in which case discovery stays + // HEAD-only exactly as before. + this.referenceResolver = new OclReferenceResolver({ + httpClient: this.httpClient, + token: options.token || null + }); + this._defaultVersionByCanonical = new Map(); + this._headMetaByCanonical = new Map(); + this._codeSystemsByCanonical = new Map(); this._idToCodeSystem = new Map(); this.sourceMetaByUrl = new Map(); @@ -73,6 +83,7 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { oclLog.info(`Fetched ${sources.length} sources`); const snapshot = this.#buildSourceSnapshot(sources); + await this.#augmentSnapshotWithDefaultVersions(snapshot); this.#applySnapshot(snapshot); oclLog.info(`Loaded ${this._codeSystemsByCanonical.size} code systems`); @@ -139,7 +150,94 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { } getSourceMetas() { - return Array.from(this.sourceMetaByUrl.values()); + // Default metas first: registerProvider's unversioned key is first-wins, so + // the release (when one exists) becomes what a versionless request gets, + // matching OCL's own resolution. HEAD variants stay reachable via |HEAD. + return [ + ...this.sourceMetaByUrl.values(), + ...this._headMetaByCanonical.values() + ]; + } + + /** + * Resolve each source's DEFAULT version via one chunked $resolveReference + * batch: OCL answers with the latest release, or HEAD when none is released. + * For sources whose default is a release, the snapshot entry becomes the + * release (CodeSystem resource, meta, versioned concepts URL) and the HEAD + * variant is kept as an extra meta, so BOTH versions get factories. + * + * Only canonicals that are new or whose listing checksum changed are + * re-resolved, so a steady-state refresh costs no extra requests. A no-op + * without a token: discovery stays HEAD-only exactly as before. + */ + async #augmentSnapshotWithDefaultVersions(snapshot) { + if (!this.referenceResolver.isEnabled()) { + return; + } + + const previousSnapshot = this._sourceStateByCanonical; + const toResolve = []; + for (const [canonicalUrl, entry] of snapshot.entries()) { + entry.baseChecksum = entry.checksum; + const previous = previousSnapshot.get(canonicalUrl); + const previousBase = previous?.baseChecksum ?? previous?.checksum ?? null; + if (!this._defaultVersionByCanonical.has(canonicalUrl) || previousBase !== entry.baseChecksum) { + toResolve.push(canonicalUrl); + } + } + + if (toResolve.length > 0) { + const results = await this.referenceResolver.resolveReferences(toResolve, { bypassCache: true }); + if (Array.isArray(results)) { + toResolve.forEach((canonicalUrl, i) => { + const r = results[i]; + if (r?.resolved && r.result?.version) { + this._defaultVersionByCanonical.set(canonicalUrl, r.result.version); + } else { + this._defaultVersionByCanonical.delete(canonicalUrl); + } + }); + } else { + // Resolver unavailable: keep whatever defaults we knew; new canonicals + // stay HEAD-only until a later cycle succeeds. + console.warn('[OCL] Default-version resolution unavailable; keeping previous defaults'); + } + } + + let releaseCount = 0; + for (const [canonicalUrl, entry] of snapshot.entries()) { + const defaultVersion = this._defaultVersionByCanonical.get(canonicalUrl); + const headVersion = entry.meta?.version || null; + if (!defaultVersion || defaultVersion === headVersion) { + continue; + } + const conceptsUrl = entry.meta?.conceptsUrl; + if (!conceptsUrl || !conceptsUrl.endsWith('concepts/')) { + continue; + } + + const baseRepo = conceptsUrl.slice(0, -'concepts/'.length); + const releaseJson = structuredClone(entry.cs.jsonObj); + releaseJson.version = defaultVersion; + const releaseCs = new CodeSystem(releaseJson, 'R5', true); + const releaseMeta = { + ...entry.meta, + version: defaultVersion, + conceptsUrl: `${baseRepo}${encodeURIComponent(defaultVersion)}/concepts/`, + codeSystem: releaseCs + }; + + entry.headMeta = entry.meta; + entry.meta = releaseMeta; + entry.cs = releaseCs; + // A release being published or deleted must surface as "changed". + entry.checksum = `${entry.baseChecksum}|default=${defaultVersion}`; + releaseCount++; + } + + if (releaseCount > 0) { + console.log(`[OCL] ${releaseCount} code system(s) defaulting to a released version (HEAD kept as |HEAD)`); + } } #scheduleRefresh() { @@ -151,14 +249,24 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { try { const sources = await this.#fetchSourcesForDiscovery(); const nextSnapshot = this.#buildSourceSnapshot(sources); + await this.#augmentSnapshotWithDefaultVersions(nextSnapshot); const changes = this.#diffSnapshots(this._sourceStateByCanonical, nextSnapshot); this.#applySnapshot(nextSnapshot); - for (const cs of changes.added || []) { + // Create factories for versions that appeared: the default (release) meta + // first so it claims the unversioned keys, then the HEAD variant. Covers + // both newly discovered sources and a release published on a known one. + for (const cs of [...(changes.added || []), ...(changes.changed || [])]) { const entry = nextSnapshot.get(cs.url); - if (entry?.meta && !OCLSourceCodeSystemFactory.hasFactory(cs.url, cs.version || null)) { - const factory = OCLSourceCodeSystemFactory.createForDiscoveredSource(this.httpClient, entry.meta); + for (const meta of [entry?.meta, entry?.headMeta]) { + if (!meta) { + continue; + } + if (OCLSourceCodeSystemFactory.hasExactFactory(meta.canonicalUrl, meta.version || null)) { + continue; + } + const factory = OCLSourceCodeSystemFactory.createForDiscoveredSource(this.httpClient, meta); if (factory) { - oclLog.info(`Factory created for newly discovered source: ${cs.url}`); + oclLog.info(`Factory created for ${meta.canonicalUrl}|${meta.version || ''}`); } } } @@ -314,6 +422,7 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { this._codeSystemsByCanonical.clear(); this._idToCodeSystem.clear(); this.sourceMetaByUrl.clear(); + this._headMetaByCanonical.clear(); this._usedIds.clear(); for (const [canonicalUrl, entry] of snapshot.entries()) { @@ -338,6 +447,9 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { this._codeSystemsByCanonical.set(canonicalUrl, cs); this._idToCodeSystem.set(cs.id, cs); this.sourceMetaByUrl.set(canonicalUrl, meta); + if (entry.headMeta) { + this._headMetaByCanonical.set(canonicalUrl, entry.headMeta); + } } this._sourceStateByCanonical = snapshot; @@ -1862,11 +1974,14 @@ class OCLSourceCodeSystemFactory extends CodeSystemFactoryProvider { } #resourceKey() { - const crypto = require('crypto'); + // Plain `system|version`, matching how hasExactFactory/#findFactory build + // their lookup keys. This used to be a SHA-256 of the same string, which + // meant exact-version lookups could NEVER match a registered factory (plain + // string vs hash) — only the unversioned `system|` alias ever worked. The + // key is only used for in-memory maps, job keys and logs, so hashing bought + // nothing and broke version-aware matching. const normalizedSystem = OCLSourceCodeSystemFactory.#normalizeSystem(this.system()); - const base = `${normalizedSystem}|${this.version() || ''}`; - const hash = crypto.createHash('sha256').update(base).digest('hex'); - return hash; + return `${normalizedSystem}|${this.version() || ''}`; } currentChecksum() { diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js index f4f91001..379169cf 100644 --- a/tx/ocl/resolve/reference-resolver.js +++ b/tx/ocl/resolve/reference-resolver.js @@ -14,6 +14,10 @@ const RESOLVE_PATH = '/$resolveReference/'; +// The live instance rejects large batches (403 somewhere between 150 and 200 +// references); 100 resolved in ~1.1s. Chunking is transparent to callers. +const MAX_BATCH_SIZE = 100; + // Org-only visibility policy: an artifact is expected to live in an organization // to be visible through the terminology service. User-owned repos (/users/...) // are experimental by convention and are excluded from discovery AND resolution. @@ -183,11 +187,16 @@ class OclReferenceResolver { } /** - * Resolve many references in one POST. Results come back in the caller's order. + * Resolve many references, chunked into POSTs of at most MAX_BATCH_SIZE. + * Results come back in the caller's order. * + * @param {object} [options] + * @param {boolean} [options.bypassCache] - re-ask OCL even for cached entries + * (the cache is still updated). Used by discovery refresh, where a source's + * default version may have changed since it was last resolved. * @returns {Promise} null when the resolver is unavailable (use fallback) */ - async resolveReferences(refs) { + async resolveReferences(refs, { bypassCache = false } = {}) { if (!this.#enabled) { return null; } @@ -203,22 +212,32 @@ class OclReferenceResolver { bodies.forEach((body, index) => { const key = cacheKey(body); - if (this.#cache.has(key)) { + if (!bypassCache && this.#cache.has(key)) { output[index] = this.#cache.get(key); } else { misses.push({ body, index }); } }); - if (misses.length === 0) { - return output; + for (let start = 0; start < misses.length; start += MAX_BATCH_SIZE) { + const chunk = misses.slice(start, start + MAX_BATCH_SIZE); + const outcome = await this.#resolveChunk(chunk, output); + if (outcome === null) { + // Resolver became unavailable; the caller falls back wholesale rather + // than acting on a half-resolved set. + return null; + } } + return output; + } + + async #resolveChunk(chunk, output) { let response; try { - response = await this.#httpClient.post(RESOLVE_PATH, misses.map(m => m.body)); + response = await this.#httpClient.post(RESOLVE_PATH, chunk.map(m => m.body)); } catch (error) { - return this.#handleError(error, misses, output); + return this.#handleError(error, chunk, output); } const payload = Array.isArray(response?.data) @@ -228,19 +247,19 @@ class OclReferenceResolver { : [response.data]; // Results are positional. On a count mismatch we cannot know which result - // belongs to which reference, so treat everything as unresolved rather than + // belongs to which reference, so treat the chunk as unresolved rather than // silently attributing a resolution to the wrong canonical. - if (payload.length !== misses.length) { + if (payload.length !== chunk.length) { this.#logger.error( - `[OCL] $resolveReference returned ${payload.length} result(s) for ${misses.length} reference(s); discarding to avoid misaligned results` + `[OCL] $resolveReference returned ${payload.length} result(s) for ${chunk.length} reference(s); discarding to avoid misaligned results` ); - for (const { body, index } of misses) { + for (const { body, index } of chunk) { output[index] = unresolved(body); } return output; } - misses.forEach(({ body, index }, position) => { + chunk.forEach(({ body, index }, position) => { let value = normalizeResult(payload[position], body); // Org-only policy: a canonical resolving to a user-owned repo is treated as // unresolved — user artifacts are experimental and not visible through the From b9a2d05032bc09439c55251f2915540efbfa8437 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 15 Jul 2026 18:28:52 -0300 Subject: [PATCH 09/13] test(ocl): close the coverage gaps found by auditing implementation vs tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An audit of "do we have tests for everything implemented?" found four behaviours proven only live (or not at all). Now unit-tested: - Global-listing discovery (cs + vs): one crawl of /sources/ / /collections/, user-owned entries filtered by the org-only policy, and the per-org enumeration exercised as the fallback when the global listing fails. - ValueSet compose source canonicals resolved in ONE $resolveReference batch (no per-source detail GETs), seeding sourceCanonicalCache, with the compose built from the returned canonical_urls. - Version mismatch on a resolved collection falls back to the search instead of serving the wrong version. - Default-version steady state: a refresh over an unchanged listing issues NO new $resolveReference calls and keeps the release/HEAD registration intact. One test-authoring note: vs-ocl normalizes conceptsUrl to an absolute URL without a trailing slash, so mocks must match by substring — an exact-path mock silently returns empty and the batch never runs, which is how the first version of the compose test failed. Still live-only (documented, not unit-tested): post-boot factory creation for a release published after startup depends on the OCLSourceCodeSystemFactory sharedI18n singleton, which unit tests here cannot set up cheaply. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cs-default-version.test.js | 16 +++ tests/ocl/ocl-discovery-global.test.js | 109 +++++++++++++++++++++ tests/ocl/ocl-vs-resolve-reference.test.js | 68 +++++++++++++ 3 files changed, 193 insertions(+) create mode 100644 tests/ocl/ocl-discovery-global.test.js diff --git a/tests/ocl/ocl-cs-default-version.test.js b/tests/ocl/ocl-cs-default-version.test.js index e8a9da69..50126da1 100644 --- a/tests/ocl/ocl-cs-default-version.test.js +++ b/tests/ocl/ocl-cs-default-version.test.js @@ -110,6 +110,22 @@ describe('OCL CodeSystem default-version resolution', () => { expect(metas.map(m => m.version)).toEqual(['HEAD']); }); + it('does not re-resolve unchanged canonicals on refresh (steady state costs nothing)', async () => { + const { provider, httpClient } = makeProvider(); + + await provider.listCodeSystems('5.0', null); + expect(httpClient.post).toHaveBeenCalledTimes(1); + + // Minute refresh with an unchanged listing: same checksum, no new resolve. + provider.getCodeSystemChanges('5.0', null); + await new Promise(resolve => setTimeout(resolve, 150)); + + expect(httpClient.get.mock.calls.filter(c => c[0] === '/sources/').length).toBeGreaterThanOrEqual(2); + expect(httpClient.post).toHaveBeenCalledTimes(1); + // The release default survived the refresh. + expect(provider.getSourceMetas().map(m => m.version)).toEqual(['20230109', 'HEAD']); + }); + it('keeps discovery working when $resolveReference is unavailable', async () => { const error = new Error('Request failed with status code 404'); error.response = { status: 404 }; diff --git a/tests/ocl/ocl-discovery-global.test.js b/tests/ocl/ocl-discovery-global.test.js new file mode 100644 index 00000000..15dba9c8 --- /dev/null +++ b/tests/ocl/ocl-discovery-global.test.js @@ -0,0 +1,109 @@ +// Global-listing discovery: one paginated crawl of /sources/ (or /collections/) +// filtered to organization-owned entries, with the per-org enumeration kept as +// fallback when the global listing is unavailable or empty. + +const { OCLCodeSystemProvider } = require('../../tx/ocl/cs-ocl'); +const { OCLValueSetProvider } = require('../../tx/ocl/vs-ocl'); + +function orgSource(id, canonical) { + return { + id, short_code: id, owner: 'MS', owner_type: 'Organization', + url: `/orgs/MS/sources/${id}/`, canonical_url: canonical, + version: 'HEAD', concepts_url: `/orgs/MS/sources/${id}/concepts/`, + checksums: { standard: 'x' }, updated_at: '2026-01-01T00:00:00Z' + }; +} + +function userSource(id, canonical) { + return { + ...orgSource(id, canonical), + owner: 'joe', owner_type: 'User', url: `/users/joe/sources/${id}/` + }; +} + +describe('CodeSystem discovery via the global listing', () => { + it('uses one global crawl and filters out user-owned sources (org-only policy)', async () => { + const provider = new OCLCodeSystemProvider({ baseUrl: 'https://ocl.example.org' }); + const get = jest.fn(async (url) => { + if (url === '/sources/') { + return { + data: { + results: [ + orgSource('A', 'http://x.org/cs/A'), + userSource('B', 'http://x.org/cs/B'), + orgSource('C', 'http://x.org/cs/C') + ], + num_found: 3 + } + }; + } + return { data: [] }; + }); + provider.httpClient = { get, post: jest.fn() }; + + const listed = await provider.listCodeSystems('5.0', null); + + const urls = listed.map(cs => cs.url).sort(); + expect(urls).toEqual(['http://x.org/cs/A', 'http://x.org/cs/C']); + // No per-org enumeration: /orgs/ was never listed. + expect(get.mock.calls.some(c => c[0] === '/orgs/')).toBe(false); + }); + + it('falls back to per-org enumeration when the global listing fails', async () => { + const provider = new OCLCodeSystemProvider({ baseUrl: 'https://ocl.example.org' }); + const get = jest.fn(async (url) => { + if (url === '/sources/') { + const error = new Error('boom'); + error.response = { status: 500 }; + throw error; + } + if (url === '/orgs/') { + return { data: [{ id: 'MS' }] }; + } + if (url === '/orgs/MS/sources/') { + return { data: { results: [orgSource('A', 'http://x.org/cs/A')], num_found: 1 } }; + } + return { data: [] }; + }); + provider.httpClient = { get, post: jest.fn() }; + + const listed = await provider.listCodeSystems('5.0', null); + + expect(listed.map(cs => cs.url)).toEqual(['http://x.org/cs/A']); + expect(get.mock.calls.some(c => c[0] === '/orgs/MS/sources/')).toBe(true); + }); +}); + +describe('ValueSet discovery via the global listing', () => { + function orgCollection(id, canonical) { + return { + id, short_code: id, owner: 'MS', owner_type: 'Organization', + url: `/orgs/MS/collections/${id}/`, canonical_url: canonical, version: 'HEAD' + }; + } + + it('uses one global crawl and filters out user-owned collections', async () => { + const provider = new OCLValueSetProvider({ baseUrl: 'https://ocl.example.org' }); + const get = jest.fn(async (url) => { + if (url === '/collections/') { + return { + data: { + results: [ + orgCollection('VC1', 'http://x.org/vs/GlobalDiscoveryVC1'), + { ...orgCollection('VC2', 'http://x.org/vs/GlobalDiscoveryVC2'), owner: 'joe', owner_type: 'User', url: '/users/joe/collections/VC2/' } + ], + num_found: 2 + } + }; + } + return { data: [] }; + }); + provider.httpClient = { get, post: jest.fn() }; + + await provider.initialize(); + + expect(provider.valueSetMap.has('http://x.org/vs/GlobalDiscoveryVC1')).toBe(true); + expect(provider.valueSetMap.has('http://x.org/vs/GlobalDiscoveryVC2')).toBe(false); + expect(get.mock.calls.some(c => c[0] === '/orgs/')).toBe(false); + }); +}); diff --git a/tests/ocl/ocl-vs-resolve-reference.test.js b/tests/ocl/ocl-vs-resolve-reference.test.js index af38d6f8..a618cc60 100644 --- a/tests/ocl/ocl-vs-resolve-reference.test.js +++ b/tests/ocl/ocl-vs-resolve-reference.test.js @@ -89,6 +89,74 @@ describe('vs-ocl $resolveReference: collection resolution', () => { expect(get.mock.calls.some(c => /\/collections\/$/.test(c[0]))).toBe(true); }); + it('resolves compose source canonicals in one batch, not one GET per source', async () => { + const CANON = 'http://x.org/vs/BatchVS'; + const post = jest.fn(async (path, body) => ({ + data: body.map(ref => { + if (typeof ref === 'string' && ref.startsWith('/orgs/MS/sources/')) { + const id = ref.split('/').filter(Boolean).pop(); + return { + reference_type: 'relative', resolved: true, + result: { url: ref, owner_type: 'Organization', type: 'Source', canonical_url: `http://x.org/cs/${id}` } + }; + } + return collectionResolve('/orgs/MS/collections/BC/', CANON); + }) + })); + // NOTE: vs-ocl normalizes conceptsUrl/expansionUrl to ABSOLUTE urls without a + // trailing slash, so the mock matches by substring, not exact path. + const get = jest.fn(async (url, config) => { + if (url === '/orgs/MS/collections/BC/') { + return { data: { id: 'BC', url: '/orgs/MS/collections/BC/', canonical_url: CANON, version: 'HEAD', name: 'BC', owner: 'MS', owner_type: 'Organization' } }; + } + if (url.includes('/expansions/')) { + return { data: {} }; // no resolved_source_versions -> fall through to source keys + } + if (url.includes('/collections/BC/concepts') && config?.params?.page !== undefined) { + const page = config.params.page || 1; + return { data: page === 1 ? [{ owner: 'MS', source: 'S1' }, { owner: 'MS', source: 'S2' }] : [] }; + } + return { data: [] }; + }); + const { provider } = makeProvider({ post, get }); + + const vs = await provider.fetchValueSet(CANON, null); + + expect(vs).toBeTruthy(); + // POST #1 resolves the ValueSet canonical; POST #2 is ONE batch with both + // source paths — no per-source detail GETs. + expect(post).toHaveBeenCalledTimes(2); + expect(post.mock.calls[1][1]).toEqual(['/orgs/MS/sources/S1/', '/orgs/MS/sources/S2/']); + expect(get.mock.calls.some(c => /\/sources\/S[12]\/$/.test(c[0]))).toBe(false); + // The compose was built from the canonicals the batch returned. + const systems = (vs.jsonObj.compose?.include || []).map(i => i.system).sort(); + expect(systems).toEqual(['http://x.org/cs/S1', 'http://x.org/cs/S2']); + // And the per-source cache was seeded for later callers. + expect(provider.sourceCanonicalCache.get('MS|S1')).toBe('http://x.org/cs/S1'); + }); + + it('falls back to the search when the resolved collection version does not match', async () => { + const CANON = 'http://x.org/vs/VersionedVS'; + const post = jest.fn(async () => ({ data: [collectionResolve('/orgs/MS/collections/VV/', CANON)] })); + const get = jest.fn(async (url) => { + if (url === '/orgs/MS/collections/VV/') { + // The repo is HEAD; the caller asked for v1.0 — mismatch. + return { data: { id: 'VV', url: '/orgs/MS/collections/VV/', canonical_url: CANON, version: 'HEAD', name: 'VV' } }; + } + if (url === '/orgs/') return { data: [] }; + return { data: [] }; + }); + const { provider } = makeProvider({ post, get }); + + const vs = await provider.fetchValueSet(CANON, '1.0'); + + // Resolver answered, the fetched collection was rejected on version, and the + // (empty) search ran — result is honestly null rather than the wrong version. + expect(post.mock.calls[0][1]).toEqual([{ url: CANON, version: '1.0' }]); + expect(get.mock.calls.some(c => c[0] === '/orgs/MS/collections/VV/')).toBe(true); + expect(vs).toBeNull(); + }); + it('ignores a resolved repo that is not a collection', async () => { // OCL resolves the canonical to a *source*, not a collection: not a ValueSet. const post = jest.fn(async () => ({ From 0b2f3512610aab9e653aa45f4b6d23e5097af48f Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Mon, 31 Aug 2026 12:48:53 -0300 Subject: [PATCH 10/13] refactor(ocl): route canonical-resolution logging through the module logger The $resolveReference / canonical-resolution work was authored before the module-logger convention landed (PR #266), so it used console.log/warn with [OCL]/[OCL-ValueSet] prefixes. Convert those 14 calls to child loggers to keep the module consistent, per the same review feedback addressed in #266: - cm-ocl.cjs: new child logger { module: 'ocl-cm' } (5 calls) - cs-ocl.cjs: existing oclLog (3 calls) - vs-ocl.cjs: existing oclVsLog (6 calls) Redundant [OCL] message prefixes dropped (the child logger tags each line with {module}). No behavior change beyond log routing; all 165 OCL tests pass. Co-Authored-By: Claude Opus 4.8 --- tx/ocl/cm-ocl.cjs | 14 ++++++++------ tx/ocl/cs-ocl.cjs | 6 +++--- tx/ocl/vs-ocl.cjs | 12 ++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/tx/ocl/cm-ocl.cjs b/tx/ocl/cm-ocl.cjs index 43a175ff..1a7b8eca 100644 --- a/tx/ocl/cm-ocl.cjs +++ b/tx/ocl/cm-ocl.cjs @@ -4,6 +4,8 @@ const { PAGE_SIZE } = require('./shared/constants'); const { createOclHttpClient } = require('./http/client'); const { fetchAllPages, extractItemsAndNext } = require('./http/pagination'); const { OclReferenceResolver, isOclRepoPath } = require('./resolve/reference-resolver'); +const Logger = require('../../library/logger'); +const oclCmLog = Logger.getInstance().child({ module: 'ocl-cm' }); const DEFAULT_MAX_SEARCH_PAGES = 10; @@ -547,8 +549,8 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { if (!this.referenceResolver.isEnabled()) { if (!this._resolverDisabledLogged) { this._resolverDisabledLogged = true; - console.log( - `[OCL] $resolveReference not in use (${this.referenceResolver.disabledReason}); using source search` + oclCmLog.info( + `$resolveReference not in use (${this.referenceResolver.disabledReason}); using source search` ); } return null; @@ -558,18 +560,18 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { try { resolved = await this.referenceResolver.resolve(systemUrl); } catch (error) { - console.warn(`[OCL] $resolveReference lookup failed for ${systemUrl}: ${error.message}`); + oclCmLog.warn(`$resolveReference lookup failed for ${systemUrl}: ${error.message}`); return null; } if (!resolved?.resolved || !resolved.repoUrl) { - console.log(`[OCL] $resolveReference did not resolve ${systemUrl}; falling back to source search`); + oclCmLog.info(`$resolveReference did not resolve ${systemUrl}; falling back to source search`); return null; } // Logged on success too: resolver and search return the same thing, so without // this there is no way -- from outside or from the logs -- to tell which path ran. - console.log(`[OCL] $resolveReference resolved ${systemUrl} -> ${resolved.repoUrl}`); + oclCmLog.info(`$resolveReference resolved ${systemUrl} -> ${resolved.repoUrl}`); // Keep the canonical<->repo caches coherent with the search path's bookkeeping. // Index under what was asked, but record OCL's own canonical_url as the repo's @@ -663,7 +665,7 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { } } } catch (error) { - console.warn(`[OCL] $resolveReference batch for source canonicals failed: ${error.message}`); + oclCmLog.warn(`$resolveReference batch for source canonicals failed: ${error.message}`); } } diff --git a/tx/ocl/cs-ocl.cjs b/tx/ocl/cs-ocl.cjs index 3106d4cc..fa4353aa 100644 --- a/tx/ocl/cs-ocl.cjs +++ b/tx/ocl/cs-ocl.cjs @@ -200,7 +200,7 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { } else { // Resolver unavailable: keep whatever defaults we knew; new canonicals // stay HEAD-only until a later cycle succeeds. - console.warn('[OCL] Default-version resolution unavailable; keeping previous defaults'); + oclLog.warn('Default-version resolution unavailable; keeping previous defaults'); } } @@ -236,7 +236,7 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { } if (releaseCount > 0) { - console.log(`[OCL] ${releaseCount} code system(s) defaulting to a released version (HEAD kept as |HEAD)`); + oclLog.info(`${releaseCount} code system(s) defaulting to a released version (HEAD kept as |HEAD)`); } } @@ -325,7 +325,7 @@ class OCLCodeSystemProvider extends AbstractCodeSystemProvider { } } } catch (error) { - console.warn(`[OCL] Global /sources/ listing failed (${error.message}); falling back to per-org discovery`); + oclLog.warn(`Global /sources/ listing failed (${error.message}); falling back to per-org discovery`); } const organizations = await this.#fetchOrganizationIds(); diff --git a/tx/ocl/vs-ocl.cjs b/tx/ocl/vs-ocl.cjs index fed9244c..23c1d370 100644 --- a/tx/ocl/vs-ocl.cjs +++ b/tx/ocl/vs-ocl.cjs @@ -1287,7 +1287,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { version ? { url: canonicalUrl, version } : canonicalUrl ); } catch (error) { - console.warn(`[OCL-ValueSet] $resolveReference lookup failed for ${canonicalUrl}: ${error.message}`); + oclVsLog.warn(`$resolveReference lookup failed for ${canonicalUrl}: ${error.message}`); return null; } @@ -1305,7 +1305,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { const response = await this.httpClient.get(resolved.repoUrl); collection = response?.data || null; } catch (error) { - console.warn(`[OCL-ValueSet] failed to fetch resolved collection ${resolved.repoUrl}: ${error.message}`); + oclVsLog.warn(`failed to fetch resolved collection ${resolved.repoUrl}: ${error.message}`); return null; } @@ -1319,7 +1319,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { return null; } - console.log(`[OCL-ValueSet] $resolveReference resolved ${canonicalUrl} -> ${resolved.repoUrl}`); + oclVsLog.info(`$resolveReference resolved ${canonicalUrl} -> ${resolved.repoUrl}`); return collection; } @@ -1407,7 +1407,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } } } catch (error) { - console.warn(`[OCL-ValueSet] Global /collections/ listing failed (${error.message}); falling back to per-org discovery`); + oclVsLog.warn(`Global /collections/ listing failed (${error.message}); falling back to per-org discovery`); } const organizations = await this.#fetchOrganizationIds(); @@ -1915,7 +1915,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { try { results = await this.referenceResolver.resolveReferences(pending.map(p => p.ref)); } catch (error) { - console.warn(`[OCL-ValueSet] $resolveReference batch failed: ${error.message}`); + oclVsLog.warn(`$resolveReference batch failed: ${error.message}`); return; } @@ -1934,7 +1934,7 @@ class OCLValueSetProvider extends AbstractValueSetProvider { } } if (resolvedCount > 0) { - console.log(`[OCL-ValueSet] $resolveReference batch resolved ${resolvedCount}/${pending.length} source canonical(s) in one request`); + oclVsLog.info(`$resolveReference batch resolved ${resolvedCount}/${pending.length} source canonical(s) in one request`); } } From 26b34e42efe112bb6ec4bc1309bd5a30070bf836 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Tue, 6 Oct 2026 14:03:48 -0300 Subject: [PATCH 11/13] =?UTF-8?q?harden($resolveReference):=20address=20re?= =?UTF-8?q?view=20findings=20#2=E2=80=93#5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security/robustness fixes from the PR #282 review (Grahame). All within tx/ocl/**. #2 repoUrl validation / SSRF surface: - normalizeResult now rejects any repoUrl that is not a safe same-host relative path (new isSafeRelativeOclPath: must start with '/', not '//', no '..'), so a surprising OCL response can never redirect an authed request (carrying our token) off-host. - isOrgOwned no longer trusts owner_type alone: it requires a valid org repo path AND (when present) owner_type === 'Organization'. Closes the hole where {owner_type:'Organization'} on a non-org/off-host url was accepted. - REPO_PATH_PATTERN tightened to /^\/orgs\/[^/]+\/(sources|collections)\/[^/]+\// and isOclRepoPath rejects any '..' (path traversal). #3 bounded cache + input caps: - the resolution cache is now a bounded LRU (DEFAULT_CACHE_LIMIT, evicts oldest), so client-supplied references can no longer grow memory without bound. - references longer than MAX_REFERENCE_LENGTH are treated as unresolved and never sent to OCL. #4 transient-failure back-off: - a 401/403 now backs the resolver off for a cooldown (DEFAULT_AUTH_BACKOFF_MS) instead of disabling it for the whole process; 404 stays permanently disabled. #5 logging: - the resolver is given the module child logger by cm-ocl/vs-ocl (was defaulting to console); [OCL] prefixes dropped (the child logger tags {module}); all client-supplied values in log lines are sanitised (safeForLog: strips control chars, caps length) to prevent log forging. Note: isOrgOwned tests updated to assert the hardened behavior (owner_type alone no longer grants access). 173 OCL tests pass. Finding #1 (public_access gate) is held pending a live $resolveReference payload capture. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-reference-resolver.test.js | 83 ++++++++++++- tx/ocl/cm-ocl.cjs | 3 +- tx/ocl/resolve/reference-resolver.js | 150 +++++++++++++++++++---- tx/ocl/vs-ocl.cjs | 3 +- 4 files changed, 209 insertions(+), 30 deletions(-) diff --git a/tests/ocl/ocl-reference-resolver.test.js b/tests/ocl/ocl-reference-resolver.test.js index 54f261ce..61da0b5e 100644 --- a/tests/ocl/ocl-reference-resolver.test.js +++ b/tests/ocl/ocl-reference-resolver.test.js @@ -97,6 +97,12 @@ describe('isOclRepoPath (org-only policy)', () => { ['/orgs/'], ['/orgs/CIEL'], ['/groups/x/sources/S/'], + // second segment must be sources|collections, not anything + ['/orgs/CIEL/mappings/M/'], + ['/orgs/CIEL/'], + // path traversal must never pass, even when the prefix looks valid + ['/orgs/CIEL/../../users/joe/sources/S/'], + ['/orgs/CIEL/sources/../../x/'], [''], [null], [undefined] @@ -106,10 +112,14 @@ describe('isOclRepoPath (org-only policy)', () => { }); describe('isOrgOwned', () => { - it('prefers an explicit owner_type', () => { - expect(isOrgOwned({ owner_type: 'Organization', url: '/users/joe/sources/S/' })).toBe(true); + it('requires a valid org repo path; owner_type cannot override it', () => { + // owner_type alone must NOT grant access: a payload claiming Organization on a + // non-org (or off-host) URL is not served. This closes the hole where a private + // repo reachable by the configured token could be surfaced publicly. + expect(isOrgOwned({ owner_type: 'Organization', url: '/users/joe/sources/S/' })).toBe(false); expect(isOrgOwned({ owner_type: 'User', url: '/orgs/A/sources/S/' })).toBe(false); - expect(isOrgOwned({ ownerType: 'Organization' })).toBe(true); + expect(isOrgOwned({ ownerType: 'Organization' })).toBe(false); // no path -> not servable + expect(isOrgOwned({ owner_type: 'Organization', url: '/orgs/A/sources/S/' })).toBe(true); }); it('falls back to the path shape when owner_type is absent', () => { @@ -560,4 +570,71 @@ describe('OclReferenceResolver error handling', () => { expect(resolver.isEnabled()).toBe(true); expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/timeout/)); }); + + it('backs off on a transient 403 and resumes after the cooldown', async () => { + const httpClient = { + post: jest + .fn() + .mockRejectedValueOnce(httpError(403)) + .mockResolvedValueOnce({ data: [oclEntry({ url: '/orgs/A/sources/S/' })] }) + }; + // Zero cooldown: the backoff window has already elapsed by the next call. + const resolver = new OclReferenceResolver({ + httpClient, token: 'Token abc', logger: silentLogger(), authBackoffMs: 0 + }); + + await expect(resolver.resolveReferences(['/orgs/A/sources/S/'])).resolves.toBeNull(); + expect(resolver.disabledReason).toBe('not authorised (403)'); + // Unlike a 404, it is not permanently disabled. + expect(resolver.isEnabled()).toBe(true); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + expect(r.resolved).toBe(true); + expect(httpClient.post).toHaveBeenCalledTimes(2); + }); +}); + +describe('OclReferenceResolver hardening', () => { + it('treats an off-host absolute repo url as unresolved (never redirects an authed request off-host)', async () => { + const httpClient = { + post: jest.fn(async () => ({ + data: [{ resolved: true, result: { url: 'https://evil.example/orgs/A/sources/S/', owner_type: 'Organization' } }] + })) + }; + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger: silentLogger() }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(false); + expect(r.repoUrl).toBeNull(); + }); + + it('treats an over-long reference as unresolved and never sends it to OCL', async () => { + const httpClient = { post: jest.fn() }; + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger: silentLogger() }); + + const longRef = `/orgs/A/sources/${'x'.repeat(3000)}/`; + const r = await resolver.resolve(longRef); + + expect(r.resolved).toBe(false); + expect(httpClient.post).not.toHaveBeenCalled(); + }); + + it('evicts the oldest entry when the cache limit is exceeded (bounded LRU)', async () => { + const httpClient = echoClient(); + const resolver = new OclReferenceResolver({ + httpClient, token: 'Token abc', logger: silentLogger(), cacheLimit: 2 + }); + + await resolver.resolve('a'); // {a} + await resolver.resolve('b'); // {a,b} + await resolver.resolve('c'); // {b,c} — 'a' evicted + httpClient.post.mockClear(); + + await resolver.resolve('c'); // still cached — no request + expect(httpClient.post).not.toHaveBeenCalled(); + + await resolver.resolve('a'); // evicted — must re-POST + expect(httpClient.post).toHaveBeenCalledTimes(1); + }); }); diff --git a/tx/ocl/cm-ocl.cjs b/tx/ocl/cm-ocl.cjs index 1a7b8eca..f2a50311 100644 --- a/tx/ocl/cm-ocl.cjs +++ b/tx/ocl/cm-ocl.cjs @@ -31,7 +31,8 @@ class OCLConceptMapProvider extends AbstractConceptMapProvider { // keeps using its existing search. this.referenceResolver = new OclReferenceResolver({ httpClient: this.httpClient, - token: options.token || null + token: options.token || null, + logger: oclCmLog }); } diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js index 379169cf..9cd47460 100644 --- a/tx/ocl/resolve/reference-resolver.js +++ b/tx/ocl/resolve/reference-resolver.js @@ -18,10 +18,26 @@ const RESOLVE_PATH = '/$resolveReference/'; // references); 100 resolved in ~1.1s. Chunking is transparent to callers. const MAX_BATCH_SIZE = 100; +// Caps so that client-supplied references cannot grow memory or outbound traffic +// without bound (the resolver is reachable from any public terminology request): +// - the cache is a bounded LRU, so distinct references evict the oldest rather +// than accumulating forever; +// - references longer than the limit are treated as unresolved and never sent to +// OCL (nothing legitimate is anywhere near this long). +const DEFAULT_CACHE_LIMIT = 5000; +const MAX_REFERENCE_LENGTH = 2048; + +// A transient 401/403 (rate limit, a brief credential hiccup, an oversized batch +// on a busy instance) backs the resolver off for a while rather than disabling it +// for the whole process life. 404 (endpoint not implemented) stays permanent. +const DEFAULT_AUTH_BACKOFF_MS = 60_000; + // Org-only visibility policy: an artifact is expected to live in an organization -// to be visible through the terminology service. User-owned repos (/users/...) -// are experimental by convention and are excluded from discovery AND resolution. -const REPO_PATH_PATTERN = /^\/orgs\/[^/]+\//; +// to be visible through the terminology service. The path must be a concrete +// /orgs//(sources|collections)// repo path — this both enforces the +// policy and keeps repoUrl a safe relative OCL path (so it can never redirect an +// authenticated request, carrying our token, to an arbitrary host). +const REPO_PATH_PATTERN = /^\/orgs\/[^/]+\/(sources|collections)\/[^/]+\//; // Reference object fields forwarded to OCL. `namespace` is intentionally absent. const BODY_FIELDS = [ @@ -38,26 +54,46 @@ const BODY_FIELDS = [ /** * True for a relative OCL repo path the terminology service may serve — i.e. an - * organization-owned one (`/orgs/CIEL/sources/CIEL/`). User-owned paths - * (`/users/joe/...`) are rejected by policy. + * organization-owned source or collection (`/orgs/CIEL/sources/CIEL/`). User-owned + * paths (`/users/joe/...`), absolute URLs, and anything containing path traversal + * are rejected. */ function isOclRepoPath(value) { - return REPO_PATH_PATTERN.test(String(value == null ? '' : value).trim()); + const s = String(value == null ? '' : value).trim(); + if (s.includes('..')) { + // No legitimate OCL repo path contains `..`; reject traversal outright. + return false; + } + return REPO_PATH_PATTERN.test(s); +} + +/** + * A repoUrl we can safely GET against the OCL base URL: a relative, same-host path + * with no traversal. Absolute or protocol-relative URLs are rejected so an authed + * request (carrying our token) can never be redirected to an arbitrary host. This + * is the SSRF guard and is deliberately separate from the org-only policy: a + * same-host `/users/...` path is "safe" here but still rejected by isOrgOwned. + */ +function isSafeRelativeOclPath(value) { + const s = String(value == null ? '' : value).trim(); + return s.startsWith('/') && !s.startsWith('//') && !s.includes('..'); } /** - * Org-only policy check for an OCL repo payload or resolve result. Prefers the - * explicit owner_type when present; falls back to the path shape. + * Org-only policy check for an OCL repo payload or resolve result. The path must + * be a valid org repo path AND, when an explicit owner_type is present, it must be + * an Organization. owner_type alone can never override the path check — a payload + * claiming owner_type:"Organization" on a non-org (or off-host) URL is not served. */ function isOrgOwned(repo) { if (!repo || typeof repo !== 'object') { return false; } - const ownerType = repo.owner_type || repo.ownerType || null; - if (ownerType) { - return ownerType === 'Organization'; + if (!isOclRepoPath(repo.url)) { + return false; } - return isOclRepoPath(repo.url); + const ownerType = repo.owner_type || repo.ownerType || null; + return ownerType ? ownerType === 'Organization' : true; } /** @@ -95,6 +131,18 @@ function cacheKey(body) { return typeof body === 'string' ? body : JSON.stringify(body); } +// Length of the reference's URL, used for the inbound length cap. +function referenceLength(body) { + return (typeof body === 'string' ? body : String(body?.url ?? '')).length; +} + +// Client input ends up in log lines; strip control characters / newlines and cap +// the length so a crafted reference cannot forge log entries or flood the log. +function safeForLog(value) { + const s = String(value == null ? '' : value).replace(/[\u0000-\u001f\u007f]+/g, ' '); + return s.length > 200 ? `${s.slice(0, 200)}…` : s; +} + function unresolved(request) { return { resolved: false, @@ -117,6 +165,16 @@ function normalizeResult(entry, request) { const result = entry.result && typeof entry.result === 'object' ? entry.result : null; const repoUrl = result && result.url ? result.url : null; + // Only ever hand back a repoUrl that is a safe same-host relative path. Anything + // absolute or off-host (or containing traversal) is treated as unresolved, so a + // surprising OCL response can never cause a caller to issue an authenticated + // request (with our token) to an arbitrary host. The org-only policy is applied + // separately by the caller (isOrgOwned), so a same-host /users/ path survives to + // there and is rejected with a logged reason. + if (repoUrl && !isSafeRelativeOclPath(repoUrl)) { + return unresolved(entry.request === undefined ? request : entry.request); + } + return { resolved: Boolean(entry.resolved) && Boolean(repoUrl), repoUrl, @@ -139,8 +197,11 @@ class OclReferenceResolver { #httpClient; #logger; #cache = new Map(); + #cacheLimit; #enabled; #disabledReason = null; + #authBackoffMs; + #backoffUntil = 0; /** * @param {object} options @@ -149,15 +210,20 @@ class OclReferenceResolver { * $resolveReference is authenticated on every OCL instance probed, while the * listing endpoints it replaces are public, so a tokenless call is a * guaranteed 401 - * @param {object} [options.logger] + * @param {object} [options.logger] - module logger; defaults to console only as + * a last resort (callers pass their child logger) + * @param {number} [options.cacheLimit] - max cached references (bounded LRU) + * @param {number} [options.authBackoffMs] - cooldown after a transient 401/403 */ - constructor({ httpClient, token = null, logger = console } = {}) { + constructor({ httpClient, token = null, logger = console, cacheLimit = DEFAULT_CACHE_LIMIT, authBackoffMs = DEFAULT_AUTH_BACKOFF_MS } = {}) { if (!httpClient) { throw new Error('OCL reference resolver requires an http client'); } this.#httpClient = httpClient; this.#logger = logger; + this.#cacheLimit = cacheLimit > 0 ? cacheLimit : DEFAULT_CACHE_LIMIT; + this.#authBackoffMs = authBackoffMs >= 0 ? authBackoffMs : DEFAULT_AUTH_BACKOFF_MS; this.#enabled = Boolean(token); if (!this.#enabled) { this.#disabledReason = 'no token configured'; @@ -165,18 +231,44 @@ class OclReferenceResolver { } isEnabled() { - return this.#enabled; + return this.#enabled && Date.now() >= this.#backoffUntil; } get disabledReason() { return this.#disabledReason; } + // Permanent: no token, or the endpoint does not exist on this instance. #disable(reason) { this.#enabled = false; this.#disabledReason = reason; } + // Transient: back off for a cooldown, then let requests resume. + #backOff(reason) { + this.#backoffUntil = Date.now() + this.#authBackoffMs; + this.#disabledReason = reason; + } + + #cacheTouch(key) { + // Re-insert to mark most-recently-used. + const value = this.#cache.get(key); + this.#cache.delete(key); + this.#cache.set(key, value); + return value; + } + + #cachePut(key, value) { + if (this.#cache.has(key)) { + this.#cache.delete(key); + } + this.#cache.set(key, value); + if (this.#cache.size > this.#cacheLimit) { + const oldest = this.#cache.keys().next().value; + this.#cache.delete(oldest); + } + } + /** * Resolve one reference. Returns null when the resolver is unavailable, so the * caller falls back to its existing path. @@ -197,7 +289,7 @@ class OclReferenceResolver { * @returns {Promise} null when the resolver is unavailable (use fallback) */ async resolveReferences(refs, { bypassCache = false } = {}) { - if (!this.#enabled) { + if (!this.isEnabled()) { return null; } @@ -211,9 +303,14 @@ class OclReferenceResolver { const misses = []; bodies.forEach((body, index) => { + // Length cap: an over-long reference is never cached nor sent to OCL. + if (referenceLength(body) > MAX_REFERENCE_LENGTH) { + output[index] = unresolved(body); + return; + } const key = cacheKey(body); if (!bypassCache && this.#cache.has(key)) { - output[index] = this.#cache.get(key); + output[index] = this.#cacheTouch(key); } else { misses.push({ body, index }); } @@ -251,7 +348,7 @@ class OclReferenceResolver { // silently attributing a resolution to the wrong canonical. if (payload.length !== chunk.length) { this.#logger.error( - `[OCL] $resolveReference returned ${payload.length} result(s) for ${chunk.length} reference(s); discarding to avoid misaligned results` + `$resolveReference returned ${payload.length} result(s) for ${chunk.length} reference(s); discarding to avoid misaligned results` ); for (const { body, index } of chunk) { output[index] = unresolved(body); @@ -266,11 +363,11 @@ class OclReferenceResolver { // terminology service. Cached: the policy outcome is deterministic. if (value.resolved && !isOrgOwned({ owner_type: value.ownerType, url: value.repoUrl })) { this.#logger.info( - `[OCL] $resolveReference resolved ${cacheKey(body)} to a user-owned repo (${value.repoUrl}); org-only policy treats it as unresolved` + `$resolveReference resolved ${safeForLog(cacheKey(body))} to a user-owned repo (${safeForLog(value.repoUrl)}); org-only policy treats it as unresolved` ); value = unresolved(body); } - this.#cache.set(cacheKey(body), value); + this.#cachePut(cacheKey(body), value); output[index] = value; }); @@ -283,15 +380,18 @@ class OclReferenceResolver { if (status === 404) { this.#disable('endpoint not implemented (404)'); this.#logger.info( - `[OCL] $resolveReference is not available on this instance (404); using listing search instead` + `$resolveReference is not available on this instance (404); using listing search instead` ); return null; } if (status === 401 || status === 403) { - this.#disable(`not authorised (${status})`); + // Transient: back off and retry after a cooldown rather than disabling for + // the life of the process. A busy public instance can 401/403 briefly (rate + // limiting, an oversized batch) without our credentials being wrong. + this.#backOff(`not authorised (${status})`); this.#logger.warn( - `[OCL] $resolveReference rejected our credentials (${status}); using listing search instead` + `$resolveReference rejected our credentials (${status}); backing off, using listing search meanwhile` ); return null; } @@ -300,14 +400,14 @@ class OclReferenceResolver { // Our request body is wrong — a bug on this side. Don't disable: a later, // well-formed batch may be fine. const detail = error?.response?.data?.detail || error.message; - this.#logger.error(`[OCL] $resolveReference rejected the request body: ${detail}`); + this.#logger.error(`$resolveReference rejected the request body: ${safeForLog(detail)}`); for (const { body, index } of misses) { output[index] = unresolved(body); } return output; } - this.#logger.warn(`[OCL] $resolveReference failed: ${error.message}`); + this.#logger.warn(`$resolveReference failed: ${safeForLog(error.message)}`); return null; } } diff --git a/tx/ocl/vs-ocl.cjs b/tx/ocl/vs-ocl.cjs index 23c1d370..22935410 100644 --- a/tx/ocl/vs-ocl.cjs +++ b/tx/ocl/vs-ocl.cjs @@ -49,7 +49,8 @@ class OCLValueSetProvider extends AbstractValueSetProvider { // token, in which case the search paths below run exactly as before. this.referenceResolver = new OclReferenceResolver({ httpClient: this.httpClient, - token: options.token || null + token: options.token || null, + logger: oclVsLog }); this.valueSetMap = new Map(); From faa712e6961103e2bb4d64707f3678d6544bf173 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 7 Oct 2026 09:13:49 -0300 Subject: [PATCH 12/13] fix(ocl): silence no-control-regex in safeForLog (intentional control-char scrub) Code Quality (eslint) on #282 flagged the control-character class in safeForLog's regex. Stripping control chars is the function's purpose, so disable the rule for that line rather than weakening the scrub. Co-Authored-By: Claude Opus 4.8 --- tx/ocl/resolve/reference-resolver.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js index 9cd47460..e7fc2791 100644 --- a/tx/ocl/resolve/reference-resolver.js +++ b/tx/ocl/resolve/reference-resolver.js @@ -139,6 +139,8 @@ function referenceLength(body) { // Client input ends up in log lines; strip control characters / newlines and cap // the length so a crafted reference cannot forge log entries or flood the log. function safeForLog(value) { + // Matching control characters is the whole point here (we are scrubbing them). + // eslint-disable-next-line no-control-regex const s = String(value == null ? '' : value).replace(/[\u0000-\u001f\u007f]+/g, ' '); return s.length > 200 ? `${s.slice(0, 200)}…` : s; } From 665690193d3ae6ecef2049ef99602b2a7cfc97e0 Mon Sep 17 00:00:00 2001 From: Italo Macedo Date: Wed, 7 Oct 2026 09:32:01 -0300 Subject: [PATCH 13/13] feat(ocl): public-access gate + least-privilege guidance (review finding #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the token-reach concern from the PR #282 review: because the configured OCL token can see every repo its account has access to, and $resolveReference answers canonicals for anonymous terminology requests, a PRIVATE repo in the token's org could be surfaced publicly. The old org-only filter checked ownership, not visibility. Layer 2 (code, enforced): a resolved repo is now served only when it is publicly viewable (public_access View/Edit; None/unknown => unresolved, fail-closed). public_access is not inlined by $resolveReference today (the verbatim captured response omits it), so for a resolved repo the resolver fetches the repo once to read it — preferring an inline value if OCL ever adds one, cached with the result, and run concurrently across a batch. A fetch error fails closed (not served). Layer 1 (operational, documented): README now advises configuring the token with least privilege (a service account that can see only public repos), as defense-in-depth on top of the code gate. Also refreshed the README resolver section (bounded LRU, length cap, 401/403 back-off vs permanent 404) to match the finding #2–#4 changes. Tests: new public-access gate cases (None filtered, GET fallback when omitted, private-on-GET not served, fail-closed on GET error) + provider resolve mocks updated to carry public_access. 178 OCL tests pass. Co-Authored-By: Claude Opus 4.8 --- tests/ocl/ocl-cm-provider.test.js | 3 +- tests/ocl/ocl-cs-default-version.test.js | 3 +- tests/ocl/ocl-reference-resolver.test.js | 86 +++++++++++++++++++++- tests/ocl/ocl-vs-resolve-reference.test.js | 3 +- tx/ocl/README.md | 25 ++++++- tx/ocl/resolve/reference-resolver.js | 63 +++++++++++++++- 6 files changed, 173 insertions(+), 10 deletions(-) diff --git a/tests/ocl/ocl-cm-provider.test.js b/tests/ocl/ocl-cm-provider.test.js index 04af708d..3ca941ea 100644 --- a/tests/ocl/ocl-cm-provider.test.js +++ b/tests/ocl/ocl-cm-provider.test.js @@ -451,7 +451,7 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { request: null, resolution_url: url, url_registry_entry: null, - result: { type: 'Source', short_code: 'S', url, canonical_url: 'http://x.org/cs', owner_type: 'Organization' } + result: { type: 'Source', short_code: 'S', url, canonical_url: 'http://x.org/cs', owner_type: 'Organization', public_access: 'View' } } ] })); @@ -538,6 +538,7 @@ describe('OCLConceptMapProvider $resolveReference integration', () => { result: { url: repo, owner_type: 'Organization', + public_access: 'View', type: 'Source', canonical_url: `http://canon.example.org${repo}` } diff --git a/tests/ocl/ocl-cs-default-version.test.js b/tests/ocl/ocl-cs-default-version.test.js index 50126da1..9c3bbde1 100644 --- a/tests/ocl/ocl-cs-default-version.test.js +++ b/tests/ocl/ocl-cs-default-version.test.js @@ -34,6 +34,7 @@ function releaseReply() { url: '/orgs/ANVISA/sources/cmed/', owner: 'ANVISA', owner_type: 'Organization', + public_access: 'View', version: '20230109', type: 'Source Version', canonical_url: CANONICAL @@ -98,7 +99,7 @@ describe('OCL CodeSystem default-version resolution', () => { data: [{ reference_type: 'canonical', resolved: true, - result: { url: '/orgs/ANVISA/sources/cmed/', owner_type: 'Organization', version: 'HEAD', type: 'Source', canonical_url: CANONICAL } + result: { url: '/orgs/ANVISA/sources/cmed/', owner_type: 'Organization', public_access: 'View', version: 'HEAD', type: 'Source', canonical_url: CANONICAL } }] })); const { provider } = makeProvider({ post }); diff --git a/tests/ocl/ocl-reference-resolver.test.js b/tests/ocl/ocl-reference-resolver.test.js index 61da0b5e..937867b7 100644 --- a/tests/ocl/ocl-reference-resolver.test.js +++ b/tests/ocl/ocl-reference-resolver.test.js @@ -22,6 +22,11 @@ function oclEntry({ canonicalUrl = 'https://terminologia.saude.gov.br/fhir/CodeSystem/BRTabelaSUS', owner = 'MS', ownerType = 'Organization', + // The live $resolveReference result does NOT carry public_access (see the + // verbatim test below). This helper adds it so the gate's inline path is taken + // and most tests need no repo GET; pass publicAccess: null to omit it and + // exercise the GET fallback, or 'None' for a private repo. + publicAccess = 'View', registryEntry = null, referenceType = 'relative', resolutionUrl = null, @@ -46,6 +51,7 @@ function oclEntry({ source_type: 'Dictionary', canonical_url: canonicalUrl, type: 'Source', + ...(publicAccess != null ? { public_access: publicAccess } : {}), checksums: { standard: '282d3c8ce440b8a03698196967042a08', smart: '90599db3f6da397c1af26baaf9467eb1' } } : null @@ -253,7 +259,11 @@ describe('OclReferenceResolver resolution', () => { checksums: { standard: '282d3c8ce440b8a03698196967042a08', smart: '90599db3f6da397c1af26baaf9467eb1' } } }]; - const httpClient = { post: jest.fn(async () => ({ data: live })) }; + // The live result omits public_access, so the gate fetches the repo to check it. + const httpClient = { + post: jest.fn(async () => ({ data: live })), + get: jest.fn(async () => ({ data: { public_access: 'View' } })) + }; const resolver = makeResolver({ httpClient }); const r = await resolver.resolve('/orgs/MS/sources/BRTabelaSUS/concepts/1948/'); @@ -351,7 +361,9 @@ describe('OclReferenceResolver resolution', () => { resolved: true, result: { url: '/orgs/A/sources/S/', canonicalUrl: 'http://a.org/cs', ownerType: 'Organization' } }] - })) + })), + // No public_access inline -> gate fetches the repo. + get: jest.fn(async () => ({ data: { public_access: 'View' } })) }; const resolver = makeResolver({ httpClient }); @@ -638,3 +650,73 @@ describe('OclReferenceResolver hardening', () => { expect(httpClient.post).toHaveBeenCalledTimes(1); }); }); + +describe('OclReferenceResolver public-access gate', () => { + it('does not serve a private repo (public_access: None), even when org-owned', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/S/', publicAccess: 'None' })] })) + }; + const logger = silentLogger(); + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(false); + expect(r.repoUrl).toBeNull(); + expect(logger.info).toHaveBeenCalledWith(expect.stringMatching(/non-public repo.*not served/)); + // No repo GET needed: public_access was inline. + expect(httpClient.get).toBeUndefined(); + }); + + it('serves a public repo and reuses the inline public_access (no repo GET)', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/S/', publicAccess: 'View' })] })), + get: jest.fn() + }; + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger: silentLogger() }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(true); + expect(httpClient.get).not.toHaveBeenCalled(); + }); + + it('fetches public_access via a repo GET when the resolve result omits it', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/S/', publicAccess: null })] })), + get: jest.fn(async () => ({ data: { public_access: 'View' } })) + }; + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger: silentLogger() }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(true); + expect(httpClient.get).toHaveBeenCalledWith('/orgs/A/sources/S/'); + }); + + it('does not serve when the repo GET says the repo is private', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/S/', publicAccess: null })] })), + get: jest.fn(async () => ({ data: { public_access: 'None' } })) + }; + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger: silentLogger() }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(false); + }); + + it('fails closed when the public_access check errors (not served)', async () => { + const httpClient = { + post: jest.fn(async () => ({ data: [oclEntry({ url: '/orgs/A/sources/S/', publicAccess: null })] })), + get: jest.fn().mockRejectedValue(new Error('network blip')) + }; + const logger = silentLogger(); + const resolver = new OclReferenceResolver({ httpClient, token: 'Token abc', logger }); + + const r = await resolver.resolve('/orgs/A/sources/S/'); + + expect(r.resolved).toBe(false); + expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/public_access check failed/)); + }); +}); diff --git a/tests/ocl/ocl-vs-resolve-reference.test.js b/tests/ocl/ocl-vs-resolve-reference.test.js index a618cc60..2a25982a 100644 --- a/tests/ocl/ocl-vs-resolve-reference.test.js +++ b/tests/ocl/ocl-vs-resolve-reference.test.js @@ -19,6 +19,7 @@ function collectionResolve(repoUrl, canonical) { url: repoUrl, canonical_url: canonical, owner_type: 'Organization', + public_access: 'View', type: 'Collection' } }; @@ -97,7 +98,7 @@ describe('vs-ocl $resolveReference: collection resolution', () => { const id = ref.split('/').filter(Boolean).pop(); return { reference_type: 'relative', resolved: true, - result: { url: ref, owner_type: 'Organization', type: 'Source', canonical_url: `http://x.org/cs/${id}` } + result: { url: ref, owner_type: 'Organization', public_access: 'View', type: 'Source', canonical_url: `http://x.org/cs/${id}` } }; } return collectionResolve('/orgs/MS/collections/BC/', CANON); diff --git a/tx/ocl/README.md b/tx/ocl/README.md index ca406dd4..01238884 100644 --- a/tx/ocl/README.md +++ b/tx/ocl/README.md @@ -48,12 +48,29 @@ ValueSets (`vs-ocl.js`): organization-owned entries) - fallback: discover orgs via `/orgs/`, then `/orgs/{org}/collections/` per org -### Visibility policy (org-only) +### Visibility policy (org-only + public-only) An artifact is expected to live in an **organization** to be visible through the terminology service. User-owned repos (`/users/{user}/...`) are experimental by convention and are excluded from both discovery and `$resolveReference` results (a canonical resolving to a user-owned repo is treated as unresolved). +In addition, a resolved repo is only served when it is **publicly viewable** +(`public_access` is `View`/`Edit`; `None`/unknown is treated as unresolved, +fail-closed). Because the configured token can see every repo its OCL account has +access to — and `$resolveReference` answers canonicals for anonymous terminology +requests — without this gate a *private* repo in the token's org could be surfaced +publicly. `public_access` is not returned inline by `$resolveReference` today, so +for a resolved repo the resolver fetches the repo once to read it (cached with the +result; concurrent across a batch). It also rejects any `repoUrl` that is not a +safe same-host relative path, so an unexpected response can never redirect an +authenticated request off-host. + +> **Operational guidance (defense-in-depth):** configure the `token=` account with +> the **least privilege** needed — ideally a service account that can see only +> public repos. The `public_access` gate enforces public-only serving in code +> regardless, but a least-privilege token means a misconfiguration can never +> expose private content. + ### Canonical resolution via `$resolveReference` With a `token=` configured on the `ocl:` source line, the providers resolve "which repo holds this canonical URL?" through OCL's @@ -64,8 +81,10 @@ canonical, and batched resolution of a collection's compose source canonicals. Without a token nothing changes: `$resolveReference` is authenticated on every instance probed (while the listing endpoints are public), so the resolver is -constructed disabled and every caller keeps its previous search path. It also -disables itself for the process on `404`/`401`/`403`. +constructed disabled and every caller keeps its previous search path. A `404` +(endpoint not implemented) disables it for the process; a transient `401`/`403` +backs it off for a cooldown and then retries. The resolution cache is a bounded +LRU and over-long references are never sent to OCL. ### CodeSystem default versions (release vs HEAD) OCL's own resolution treats a source's **latest release** as its default version diff --git a/tx/ocl/resolve/reference-resolver.js b/tx/ocl/resolve/reference-resolver.js index e7fc2791..80fd64f5 100644 --- a/tx/ocl/resolve/reference-resolver.js +++ b/tx/ocl/resolve/reference-resolver.js @@ -79,6 +79,17 @@ function isSafeRelativeOclPath(value) { return s.startsWith('/') && !s.startsWith('//') && !s.includes('..'); } +/** + * Whether an OCL repo's public_access makes it publicly viewable. OCL uses + * 'View' | 'Edit' | 'None' ('None' = private, members only). Fail-closed: + * anything that is not an explicit public value (including null/unknown) is NOT + * public, so a repo is only served when we can positively confirm it is. + */ +function isPublicAccess(value) { + const s = String(value == null ? '' : value).trim().toLowerCase(); + return s === 'view' || s === 'edit'; +} + /** * Org-only policy check for an OCL repo payload or resolve result. The path must * be a valid org repo path AND, when an explicit owner_type is present, it must be @@ -151,6 +162,7 @@ function unresolved(request) { repoUrl: null, canonical: null, ownerType: null, + publicAccess: null, resolutionUrl: null, registryEntry: null, referenceType: null, @@ -185,6 +197,10 @@ function normalizeResult(entry, request) { // authoritative, the request is just whatever spelling the caller used. canonical: result ? (result.canonical_url || result.canonicalUrl || null) : null, ownerType: result ? (result.owner_type || result.ownerType || null) : null, + // public_access is not in the $resolveReference result today (the live capture + // omits it), so this is usually null and the repo is fetched to check — kept + // anyway so the gate is free the day OCL does inline it. + publicAccess: result ? (result.public_access ?? result.publicAccess ?? null) : null, resolutionUrl: entry.resolution_url || null, // Kept even though every observed response so far carries null: whether a URL // Registry entry was involved is exactly what the OCL-team discussion needs. @@ -358,7 +374,8 @@ class OclReferenceResolver { return output; } - chunk.forEach(({ body, index }, position) => { + // Pass 1 (sync): normalize + org-only policy. + const prelim = chunk.map(({ body, index }, position) => { let value = normalizeResult(payload[position], body); // Org-only policy: a canonical resolving to a user-owned repo is treated as // unresolved — user artifacts are experimental and not visible through the @@ -369,13 +386,55 @@ class OclReferenceResolver { ); value = unresolved(body); } + return { body, index, value }; + }); + + // Pass 2 (async): public-access gate. A private repo (public_access other than + // View/Edit) that the configured token can see must NOT be served through the + // public terminology server, so anything not positively confirmed public is + // dropped. public_access is not in the resolve result today, so for a resolved + // repo we fetch it once (cached via the result below); done concurrently. + await Promise.all(prelim.map(async entry => { + if (!entry.value.resolved) { + return; + } + const access = await this.#publicAccessOf(entry.value); + if (!isPublicAccess(access)) { + this.#logger.info( + `$resolveReference resolved ${safeForLog(cacheKey(entry.body))} to a non-public repo ` + + `(${safeForLog(entry.value.repoUrl)}, public_access=${safeForLog(access)}); not served` + ); + entry.value = unresolved(entry.body); + } + })); + + for (const { body, index, value } of prelim) { this.#cachePut(cacheKey(body), value); output[index] = value; - }); + } return output; } + // The repo's public_access, preferring the value inlined in the resolve result + // and otherwise fetching the repo once. Returns null (→ fail-closed, not served) + // when it cannot be determined, including on a fetch error. + async #publicAccessOf(value) { + if (value.publicAccess != null) { + return value.publicAccess; + } + try { + const response = await this.#httpClient.get(value.repoUrl); + const repo = response?.data && typeof response.data === 'object' ? response.data : null; + return repo ? (repo.public_access ?? repo.publicAccess ?? null) : null; + } catch (error) { + this.#logger.warn( + `public_access check failed for ${safeForLog(value.repoUrl)}: ${safeForLog(error.message)}` + ); + return null; + } + } + #handleError(error, misses, output) { const status = error?.response?.status;