🧪 Auto-PR: Merge develop → test

backend/package.json

@@ -1,7 +1,7 @@
 {
   "name": "proxy-smart-backend",
   "displayName": "Proxy Smart Backend",
-  "version": "0.3.16-beta.202608161355.6a472910c",
+  "version": "0.3.16-alpha.202608161817.167c04f59",
   "type": "module",
   "scripts": {
     "test": "bun test --isolate",

backend/src/lib/admin-config-store.ts

@@ -51,6 +51,9 @@ import { getSharedPool, hasDatabaseUrl } from './pg-pool'
  */
 const CACHE_TTL_MS = 5_000
 
+/** Compare-and-set attempts before giving up. Contention here is admins clicking, not a hot path. */
+const MUTATE_MAX_ATTEMPTS = 5
+
 /** A JSON-serialisable admin config value. */
 export type AdminConfigValue = Record<string, unknown>
 
@@ -62,11 +65,28 @@ export type AdminConfigValue = Record<string, unknown>
  * with a custom backend (e.g. a failing primary) to exercise the resilient
  * fallback path without a real database.
  */
+/** A stored value together with the revision it was read at, for compare-and-set. */
+export interface VersionedAdminConfigValue {
+  value: AdminConfigValue | null
+  /** 0 when the key does not exist yet, so creating is just a compare-and-set from 0. */
+  version: number
+}
+
 export interface AdminConfigBackend {
   /** Load the raw value for a key, or null if it has never been written. */
   load(key: string): Promise<AdminConfigValue | null>
   /** Persist the value for a key. */
   store(key: string, value: AdminConfigValue): Promise<void>
+  /**
+   * Read a value together with its revision. Optional: a backend without
+   * compare-and-set omits this pair and {@link AdminConfigStore.mutate} serialises in-process.
+   */
+  loadVersioned?(key: string): Promise<VersionedAdminConfigValue>
+  /**
+   * Persist only while the stored revision is still `expectedVersion`.
+   * False means another writer got there first and the caller must re-read and retry.
+   */
+  storeIfVersion?(key: string, value: AdminConfigValue, expectedVersion: number): Promise<boolean>
 }
 
 // ── File backend (local dev / no DATABASE_URL) ────────────────────────────────
@@ -118,6 +138,11 @@ class PostgresAdminConfigBackend implements AdminConfigBackend {
         updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW()
       );
     `)
+    // Added after the table existed, so it is a separate idempotent statement rather than a
+    // migration. `version` is what makes a mutation safe when more than one task is writing.
+    await getSharedPool().query(
+      'ALTER TABLE admin_config ADD COLUMN IF NOT EXISTS version BIGINT NOT NULL DEFAULT 1',
+    )
     this.initialized = true
     logger.info('security', 'PostgreSQL admin_config table initialized')
   }
@@ -137,15 +162,60 @@ class PostgresAdminConfigBackend implements AdminConfigBackend {
     await this.initialize()
     await getSharedPool().query(
       `
-      INSERT INTO admin_config (config_key, value, updated_at)
-      VALUES ($1, $2, NOW())
+      INSERT INTO admin_config (config_key, value, updated_at, version)
+      VALUES ($1, $2, NOW(), 1)
       ON CONFLICT (config_key) DO UPDATE SET
         value = EXCLUDED.value,
-        updated_at = NOW()
+        updated_at = NOW(),
+        version = admin_config.version + 1
       `,
       [key, JSON.stringify(value)],
     )
   }
+
+  async loadVersioned(key: string): Promise<VersionedAdminConfigValue> {
+    await this.initialize()
+    const result = await getSharedPool().query(
+      'SELECT value, version FROM admin_config WHERE config_key = $1',
+      [key],
+    )
+    if (result.rows.length === 0) return { value: null, version: 0 }
+    // BIGINT arrives as a string from pg; a config revision never approaches Number.MAX_SAFE_INTEGER.
+    return {
+      value: result.rows[0].value as AdminConfigValue,
+      version: Number(result.rows[0].version),
+    }
+  }
+
+  /**
+   * The whole point of this class for concurrent writers: the UPDATE only lands while the row is
+   * still at the revision the caller read. Version 0 means "did not exist", so the insert is
+   * conditional on nobody else having created it first.
+   */
+  async storeIfVersion(key: string, value: AdminConfigValue, expectedVersion: number): Promise<boolean> {
+    await this.initialize()
+    if (expectedVersion === 0) {
+      const inserted = await getSharedPool().query(
+        `
+        INSERT INTO admin_config (config_key, value, updated_at, version)
+        VALUES ($1, $2, NOW(), 1)
+        ON CONFLICT (config_key) DO NOTHING
+        `,
+        [key, JSON.stringify(value)],
+      )
+      return (inserted.rowCount ?? 0) > 0
+    }
+
+    const updated = await getSharedPool().query(
+      `
+      UPDATE admin_config
+         SET value = $2, updated_at = NOW(), version = version + 1
+       WHERE config_key = $1 AND version = $3
+      `,
+      [key, JSON.stringify(value), expectedVersion],
+    )
+    return (updated.rowCount ?? 0) > 0
+  }
 }
 
 // ── Resilient backend (Postgres primary, file fallback) ──────────────────────
@@ -230,6 +300,8 @@ interface CacheEntry {
 export class AdminConfigStore {
   private readonly backend: AdminConfigBackend
   private readonly cache = new Map<string, CacheEntry>()
+  /** One promise chain per key, so same-task mutations queue instead of interleaving. */
+  private readonly mutations = new Map<string, Promise<void>>()
   /** True when persistence is durable across tasks/restarts (Postgres). */
   readonly durable: boolean
 
@@ -326,6 +398,60 @@ export class AdminConfigStore {
     await this.backend.store(key, serialisable)
   }
 
+  /**
+   * Change a config value SAFELY when more than one task can write it.
+   *
+   * `set` is last-writer-wins over a whole document, and every caller that appends to a list was
+   * doing read-then-set across a 5-second cache: publishing one app silently unpublished another,
+   * because the writing task had never seen it. This reads the CURRENT value, applies `update`, and
+   * writes only while the revision is unchanged — retrying when it is not.
+   *
+   * Falls back to a serialised read-modify-write when the backend cannot compare-and-set (the file
+   * backend, which is single-task by definition). In-process calls are serialised per key either
+   * way, so two requests on the SAME task cannot interleave.
+   */
+  async mutate<T extends object>(
+    key: string,
+    defaults: T,
+    merge: (defaults: T, raw: AdminConfigValue | null) => T,
+    update: (current: T) => T,
+  ): Promise<T> {
+    const run = async (): Promise<T> => {
+      const { loadVersioned, storeIfVersion } = this.backend
+      if (!loadVersioned || !storeIfVersion) {
+        const raw = await this.backend.load(key)
+        const next = update(merge(defaults, raw))
+        await this.set(key, next)
+        return next
+      }
+
+      for (let attempt = 0; attempt < MUTATE_MAX_ATTEMPTS; attempt++) {
+        const { value, version } = await loadVersioned.call(this.backend, key)
+        const next = update(merge(defaults, value))
+        const written = await storeIfVersion.call(this.backend, key, next as AdminConfigValue, version)
+        if (written) {
+          this.cache.set(key, { value: next as AdminConfigValue, loadedAt: Date.now(), refreshing: false })
+          return next
+        }
+        // Someone else wrote between our read and our write; re-read and reapply on their result.
+        logger.debug('security', 'admin config mutation retrying after a concurrent write', { key, attempt })
+      }
+
+      throw new Error(`Could not update admin config '${key}': too many concurrent writers`)
+    }
+
+    // Chain per key so concurrent callers in THIS task queue rather than race each other.
+    const queued = (this.mutations.get(key) ?? Promise.resolve()).then(run, run)
+    this.mutations.set(
+      key,
+      queued.then(
+        () => undefined,
+        () => undefined,
+      ),
+    )
+    return queued
+  }
+
   /**
    * Force a synchronous-from-the-caller's-view refresh of a key. Used by tests
    * and by callers that need to guarantee they read the latest persisted state.

backend/src/lib/app-store-config.ts

@@ -87,26 +87,55 @@ export function getAppStoreConfig(): AppStoreConfig {
   return syncedStore().getConfig()
 }
 
-export function setHiddenAppIds(ids: string[]): AppStoreConfig {
-  return syncedStore().setHiddenAppIds(ids)
-}
-
 export function getPublishedApps(): PublishedApp[] {
   return syncedStore().getPublishedApps()
 }
 
-export function publishApp(app: PublishedApp): AppStoreConfig {
-  return syncedStore().publishApp(app)
+/**
+ * Every mutation goes through the shared store's compare-and-set.
+ *
+ * The package store mutates a whole document in memory and persists it last-writer-wins, which loses
+ * updates the moment two tasks write: publishing one app unpublished another that the writing task
+ * had never read. These build the next document from the CURRENT one and retry on conflict, so a
+ * concurrent publish is merged rather than dropped.
+ */
+async function mutateConfig(update: (current: AppStoreConfig) => AppStoreConfig): Promise<AppStoreConfig> {
+  const next = await adminConfigStore.mutate<AppStoreConfig>(CONFIG_KEY, DEFAULTS, mergeConfig, update)
+  // Keep the package store's in-process view in step for the sync readers above.
+  store.reload()
+  return next
+}
+
+export function setHiddenAppIds(ids: string[]): Promise<AppStoreConfig> {
+  return mutateConfig((current) => ({ ...current, hiddenAppIds: [...ids] }))
+}
+
+export function publishApp(app: PublishedApp): Promise<AppStoreConfig> {
+  return mutateConfig((current) => ({
+    ...current,
+    // Replace an existing entry in place rather than appending a duplicate.
+    publishedApps: [...current.publishedApps.filter((a) => a.clientId !== app.clientId), app],
+  }))
 }
 
-export function unpublishApp(clientId: string): AppStoreConfig {
-  return syncedStore().unpublishApp(clientId)
+export function unpublishApp(clientId: string): Promise<AppStoreConfig> {
+  return mutateConfig((current) => ({
+    ...current,
+    publishedApps: current.publishedApps.filter((a) => a.clientId !== clientId),
+  }))
 }
 
-export function hideApp(appId: string): AppStoreConfig {
-  return syncedStore().hideApp(appId)
+export function hideApp(appId: string): Promise<AppStoreConfig> {
+  return mutateConfig((current) =>
+    current.hiddenAppIds.includes(appId)
+      ? current
+      : { ...current, hiddenAppIds: [...current.hiddenAppIds, appId] },
+  )
 }
 
-export function showApp(appId: string): AppStoreConfig {
-  return syncedStore().showApp(appId)
+export function showApp(appId: string): Promise<AppStoreConfig> {
+  return mutateConfig((current) => ({
+    ...current,
+    hiddenAppIds: current.hiddenAppIds.filter((id) => id !== appId),
+  }))
 }

backend/src/lib/launch-context-store.ts

@@ -53,6 +53,12 @@ export interface LaunchSession {
   fhirContext?: string
   /** Whether patient picker is required (standalone launch without pre-set context) */
   needsPatientPicker?: boolean
+  /**
+   * Set ONLY by the callback gate, once the user has been established as a practitioner.
+   * `/auth/patient-search` refuses without it, so the directory cannot be reached by holding a
+   * session key alone.
+   */
+  pickerAllowed?: boolean
   /** FHIR server base URL from the aud/resource parameter (e.g., "https://proxy.example.com/proxy-smart-backend/hapi-fhir-server/R4") */
   aud?: string
   /** Keycloak user sub (populated after KC callback) */

backend/src/routes/admin/app-store.ts

@@ -131,8 +131,8 @@ export const appStoreAdminRoutes = new Elysia({ prefix: '/app-store' })
     },
   })
   // POST /admin/app-store/:appId/hide — hide an app from the public store
-  .post('/:appId/hide', ({ params }) => {
-    const config = hideApp(params.appId)
+  .post('/:appId/hide', async ({ params }) => {
+    const config = await hideApp(params.appId)
     logger.server.info(`App store: hid app "${params.appId}"`)
     return { success: true, hiddenAppIds: config.hiddenAppIds, updatedAt: config.updatedAt }
   }, {
@@ -145,8 +145,8 @@ export const appStoreAdminRoutes = new Elysia({ prefix: '/app-store' })
     },
   })
   // POST /admin/app-store/:appId/show — show an app in the public store
-  .post('/:appId/show', ({ params }) => {
-    const config = showApp(params.appId)
+  .post('/:appId/show', async ({ params }) => {
+    const config = await showApp(params.appId)
     logger.server.info(`App store: showed app "${params.appId}"`)
     return { success: true, hiddenAppIds: config.hiddenAppIds, updatedAt: config.updatedAt }
   }, {
@@ -159,8 +159,8 @@ export const appStoreAdminRoutes = new Elysia({ prefix: '/app-store' })
     },
   })
   // POST /admin/app-store/publish — publish a registered app to the store
-  .post('/publish', ({ body }) => {
-    const config = publishApp(body)
+  .post('/publish', async ({ body }) => {
+    const config = await publishApp(body)
     logger.server.info(`App store: published registered app "${body.clientId}" (${body.name})`)
     return { success: true, publishedApps: config.publishedApps, updatedAt: config.updatedAt }
   }, {
@@ -175,8 +175,8 @@ export const appStoreAdminRoutes = new Elysia({ prefix: '/app-store' })
     },
   })
   // POST /admin/app-store/:appId/unpublish — remove a registered app from the store
-  .post('/:appId/unpublish', ({ params }) => {
-    const config = unpublishApp(params.appId)
+  .post('/:appId/unpublish', async ({ params }) => {
+    const config = await unpublishApp(params.appId)
     logger.server.info(`App store: unpublished registered app "${params.appId}"`)
     return { success: true, publishedApps: config.publishedApps, updatedAt: config.updatedAt }
   }, {

backend/src/routes/admin/healthcare-users.ts

@@ -548,8 +548,8 @@ export const healthcareUsersRoutes = new Elysia({ prefix: '/healthcare-users' })
         federatedIdentityId: params.provider,
         federatedIdentity: {
           identityProvider: params.provider,
-          userId: linkBody.userId,
-          userName: linkBody.userName
+          userId: linkBody.providerUserId,
+          userName: linkBody.providerUserName
         }
       })
       return { success: true, message: `Linked identity provider '${params.provider}'` }

backend/src/routes/auth/oauth.ts

@@ -261,6 +261,14 @@ export const oauthRoutes = new Elysia({ tags: ['authentication'] })
       return redirect(errorUrl.href)
     }
 
+    // Same gate as the search endpoint: the picker UI is only for users cleared to use it.
+    if (!session.pickerAllowed && session.needsPatientPicker) {
+      const errorUrl = new URL(`${config.baseUrl}/patient-picker/`)
+      errorUrl.searchParams.set('error', 'access_denied')
+      errorUrl.searchParams.set('error_description', 'Selecting a patient requires a practitioner account.')
+      return redirect(errorUrl.href)
+    }
+
     // Guard: if a patient was already selected (e.g. user hit browser back), skip the picker
     if (!session.needsPatientPicker && session.patient) {
       const clientUrl = new URL(session.clientRedirectUri)
@@ -321,6 +329,19 @@ export const oauthRoutes = new Elysia({ tags: ['authentication'] })
       return { error: 'session_expired', error_description: 'Session expired. Please restart the authorization flow.' }
     }
 
+    /*
+     * A session key alone is not permission to read the patient directory. `pickerAllowed` is set
+     * only by the callback gate, and only once the user was established as a practitioner — so this
+     * endpoint cannot be reached by a patient who happens to hold a launch session.
+     */
+    if (!session.pickerAllowed) {
+      logger.auth.warn('Patient search refused: session was never cleared for the picker', {
+        clientId: session.clientId,
+      })
+      set.status = 403
+      return { error: 'access_denied', error_description: 'Selecting a patient requires a practitioner account.' }
+    }
+
     // Parse server_name and fhir_version from the session aud URL
     // aud format: {baseUrl}/{appName}/{server_name}/{fhir_version}
     const aud = session.aud

backend/src/schemas/admin/users.ts

@@ -17,9 +17,15 @@ export const FederatedIdentity = t.Object({
   userName: t.String({ description: 'Username at the identity provider' })
 }, { title: 'FederatedIdentity' })
 
+/**
+ * `providerUserId` is deliberately NOT `userId`: the local Keycloak user is already a PATH parameter
+ * of the same name, and a generated client that flattens path and body — the MCP tool surface does —
+ * collapses the two and silently drops one. That made this endpoint uncallable, so a user whose
+ * broker link was missing could not be repaired without deleting the account.
+ */
 export const LinkFederatedIdentityRequest = t.Object({
-  userId: t.String({ description: 'User ID at the identity provider' }),
-  userName: t.String({ description: 'Username at the identity provider' })
+  providerUserId: t.String({ description: "Subject (`sub`) this user has AT the identity provider" }),
+  providerUserName: t.String({ description: 'Username at the identity provider' })
 }, { title: 'LinkFederatedIdentityRequest' })
 
 /**