fix(api): resolve the instrument subject filter without a relation join

.agents/docs/architecture/auth-and-permissions.md

@@ -70,6 +70,16 @@ available in this codebase. Check the `where` clause of every query you add or t
 that the controller actually forwards `@CurrentUser('ability')` — `EntityOperationOptions.ability`
 is optional, so a controller that simply never passes it compiles and runs.
 
+**A defined ability holding no rule for the queried subject does not produce a deny-all filter —
+CASL's `accessibleBy` throws `ForbiddenError`.** That error is not an `HttpException`, so the
+global exception filter renders it as a 500. It is reachable whenever the route guard names a
+different subject than the service queries: a STANDARD user passes a `read Instrument` guard but
+holds no rule on `InstrumentRecord`, so a record query made on their behalf throws. The throw is
+deliberate — `accessibleQuery` is kept as-is rather than returning a deny-all filter — so where
+such a caller is legitimate, guard the query with `ability.can(action, subject)` and return the
+empty result instead; `findInstrumentIdsBySubject` in `src/instruments/instruments.service.ts` is
+the reference example.
+
 One call site uses `{ ...accessibleQuery(...), id: subjectId }` instead of `AND: [...]`
 (`src/subjects/subjects.service.ts`). Spreading merges keys and will silently lose a condition if
 the generated query and the literal share one. Prefer `AND`.

apps/api/AGENTS.md

@@ -83,6 +83,14 @@ restriction**, which reads clinical data across every group. Nothing catches thi
 eslint, not a passing test suite. It is the single highest-severity mistake available in this
 codebase, so verify the `where` clause of any query you add or edit.
 
+The defined case has the opposite failure mode: **an ability holding no rule at all for the queried
+subject makes `accessibleQuery` throw CASL's `ForbiddenError`**, which is not an `HttpException`
+and so surfaces as a 500. This is reachable whenever a route's guard names one subject but the
+service queries another — a STANDARD user passes `read Instrument` yet holds no `read` rule on
+`InstrumentRecord`. That behaviour is deliberate and stays; where such a caller is legitimate,
+short-circuit before the query with `ability.can(action, subject)` and return the empty result —
+see `findInstrumentIdsBySubject` in `src/instruments/instruments.service.ts`.
+
 Granting a new `action`/`subject` pair means editing `src/auth/ability.factory.ts` and adding tests
 for **both** the allow and the deny case — see `src/auth/__tests__/ability.factory.test.ts`.
 

apps/api/src/instruments/__tests__/instruments.service.spec.ts

@@ -9,6 +9,9 @@ import type { SeriesInstrument } from '@opendatacapture/runtime-core';
 import type { WithID } from '@opendatacapture/schemas/core';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
+import { AbilityFactory } from '@/auth/ability.factory';
+import { accessibleQuery, createAppAbility } from '@/auth/ability.utils';
+
 import { InstrumentsService } from '../instruments.service';
 
 import type { InstrumentVirtualizationContext } from '../instruments.service';
@@ -644,4 +647,79 @@ describe('InstrumentsService', () => {
       ]);
     });
   });
+
+  describe('find', () => {
+    beforeEach(() => {
+      instrumentModel.findMany.mockResolvedValue([]);
+      instrumentRecordModel.findMany.mockResolvedValue([]);
+      vi.spyOn(instrumentsService as any, 'instantiate').mockResolvedValue([]);
+    });
+
+    it('should not query records when no subject is given, so the unfiltered listing costs one query', async () => {
+      await instrumentsService.find();
+      expect(instrumentRecordModel.findMany).not.toHaveBeenCalled();
+    });
+
+    // A `records: { some: ... }` relation filter compiles to a $lookup that materialises every
+    // record belonging to an instrument, which mongodb aborts past 100 MiB. The subject's own
+    // records are queried instead, so the work is bounded by the subject rather than the instrument.
+    it('should resolve the subject filter against records rather than joining from instruments', async () => {
+      instrumentRecordModel.findMany.mockResolvedValueOnce([{ instrumentId: 'id-1' }, { instrumentId: 'id-2' }] as any);
+
+      await instrumentsService.find({ subjectId: 'subject-1' });
+
+      expect(instrumentRecordModel.findMany).toHaveBeenCalledWith({
+        distinct: ['instrumentId'],
+        select: { instrumentId: true },
+        where: { AND: [{}, { subjectId: 'subject-1' }] }
+      });
+      expect(instrumentModel.findMany.mock.lastCall?.[0]).toMatchObject({
+        where: { AND: expect.arrayContaining([{ id: { in: ['id-1', 'id-2'] } }]) }
+      });
+      expect(JSON.stringify(instrumentModel.findMany.mock.lastCall?.[0])).not.toContain('records');
+    });
+
+    it('should constrain the record lookup to what the caller may read, so the filter cannot be resolved from other groups records', async () => {
+      // Conditions are what make this meaningful: an unconditional read rule yields `{}`, which is
+      // indistinguishable from the ability never having been applied.
+      const ability = createAppAbility([
+        { action: 'read', conditions: { groupId: { in: ['group-1'] } }, subject: 'InstrumentRecord' },
+        { action: 'read', subject: 'Instrument' }
+      ]);
+
+      await instrumentsService.find({ subjectId: 'subject-1' }, { ability });
+
+      const [call] = instrumentRecordModel.findMany.mock.lastCall as [{ where: { AND: unknown[] } }];
+      expect(call.where.AND[0]).toStrictEqual(accessibleQuery(ability, 'read', 'InstrumentRecord'));
+    });
+
+    // Built through the factory rather than by hand: a hand-built ability tends to include a
+    // `read InstrumentRecord` rule, and it is the absence of one that breaks. A STANDARD user holds
+    // `create` but not `read`, and this route is gated on `read Instrument`, which they do hold.
+    it('should resolve for a caller who may read no records, rather than failing the request', async () => {
+      const abilityFactory = new AbilityFactory(MockFactory.createMock(LoggingService) as unknown as LoggingService);
+      const ability = abilityFactory.createForPayload({
+        basePermissionLevel: 'STANDARD',
+        groups: [{ id: 'group-1' }],
+        id: 'user-1'
+      } as any);
+
+      await expect(instrumentsService.find({ subjectId: 'subject-1' }, { ability })).resolves.toStrictEqual([]);
+
+      expect(instrumentRecordModel.findMany).not.toHaveBeenCalled();
+      expect(instrumentModel.findMany.mock.lastCall?.[0]).toMatchObject({
+        where: { AND: expect.arrayContaining([{ id: { in: [] } }]) }
+      });
+    });
+
+    it('should return nothing when the subject has no records', async () => {
+      instrumentRecordModel.findMany.mockResolvedValueOnce([]);
+
+      await instrumentsService.find({ subjectId: 'subject-with-no-records' });
+
+      expect(instrumentModel.findMany.mock.lastCall?.[0]).toMatchObject({
+        where: { AND: expect.arrayContaining([{ id: { in: [] } }]) }
+      });
+    });
+  });
 });

apps/api/src/instruments/instruments.service.ts

@@ -36,6 +36,7 @@ import type {
 import { pick } from 'lodash-es';
 
 import { accessibleQuery } from '@/auth/ability.utils';
+import type { AppAbility } from '@/auth/auth.types';
 import type { EntityOperationOptions } from '@/core/types';
 
 import { CreateInstrumentDto } from './dto/create-instrument.dto';
@@ -261,18 +262,20 @@ export class InstrumentsService {
     { ability }: EntityOperationOptions = {},
     groupIds?: string[]
   ): Promise<WithID<SomeInstrument<TKind>>[]> {
+    // Resolved against InstrumentRecord rather than expressed as a `records: { some: ... }` relation
+    // filter. On mongodb prisma compiles that filter into a $lookup which materialises *every* record
+    // belonging to an instrument into one array before applying the predicate, and mongodb caps a
+    // single document's $lookup output at 100 MiB. A busy instrument therefore fails the whole query
+    // outright once it passes that threshold. Querying the child collection is bounded by the
+    // subject's own records instead of the instrument's.
+    const subjectInstrumentIds = query.subjectId
+      ? await this.findInstrumentIdsBySubject(query.subjectId, ability)
+      : null;
+
     const instruments = await this.instrumentModel.findMany({
       where: {
         AND: [
-          {
-            records: query.subjectId
-              ? {
-                  some: {
-                    subjectId: query.subjectId
-                  }
-                }
-              : undefined
-          },
+          subjectInstrumentIds ? { id: { in: subjectInstrumentIds } } : {},
           query.seriesGroupId
             ? {
                 OR: [
@@ -520,6 +523,29 @@ export class InstrumentsService {
     >;
   }
 
+  /**
+   * The ids of the instruments the subject has at least one record for.
+   *
+   * Scoped to the records the caller may read, so the filter cannot be resolved from records outside
+   * their groups — the ids feed straight into the instrument query, so an unscoped lookup would
+   * disclose which instruments a subject has been administered elsewhere.
+   */
+  private async findInstrumentIdsBySubject(subjectId: string, ability?: AppAbility): Promise<string[]> {
+    // `accessibleQuery` throws rather than returning a restrictive filter when the ability holds no
+    // rule for the subject at all, and a STANDARD user holds `create` but not `read` on
+    // InstrumentRecord. This route is gated on `read Instrument`, which they do hold, so they reach
+    // here; no readable records means no instruments qualify.
+    if (ability && !ability.can('read', 'InstrumentRecord')) {
+      return [];
+    }
+    const records = await this.instrumentRecordModel.findMany({
+      distinct: ['instrumentId'],
+      select: { instrumentId: true },
+      where: { AND: [accessibleQuery(ability, 'read', 'InstrumentRecord'), { subjectId }] }
+    });
+    return records.map((record) => record.instrumentId);
+  }
+
   /**
    * The title of an existing series instrument that contains the exact same ordered sequence of items as
    * `items`, or `null` when none exists. Used to warn before creating a redundant series.

testing/src/specs/authorization.spec.ts

@@ -39,5 +39,44 @@ test.describe('authorization', () => {
       await page.goto('/session/remote-assignment');
       await expect(page).toHaveURL('/session/start-session');
     });
+
+    // `GET /v1/instruments/info` is gated on `read Instrument`, which a standard user holds, but
+    // resolving `subjectId` reads instrument records, which they do not. Scoping that lookup to the
+    // caller must answer with an empty list rather than failing the request.
+    //
+    // The defect this endpoint was changed for -- a mongodb $lookup exceeding its 100 MiB per-document
+    // ceiling once one instrument holds ~182,000 records -- cannot be reproduced at this tier. That
+    // gap is deliberate; the evidence for it is in the PR, measured against a live instance.
+    test('should answer the subject-filtered instrument list for a caller who may read no records', async ({
+      apiRequestContext,
+      roleAccount
+    }) => {
+      const { accessToken } = await roleAccount('STANDARD');
+
+      const response = await apiRequestContext.get('/api/v1/instruments/info?subjectId=any-subject', {
+        headers: { Authorization: `Bearer ${accessToken}` }
+      });
+
+      expect(response.status()).toBe(200);
+      expect(await response.json()).toStrictEqual([]);
+    });
+  });
+
+  // The populated case -- a group manager reaching /datahub/$subjectId/table and picking an
+  // instrument from the list -- is covered end to end by `instrument-completion.spec.ts`, which
+  // administers one first so the list has something in it. This case only pins the contract for a
+  // subject with no visible records: an empty list, not an error.
+  test('should answer the subject-filtered instrument list for a group manager rather than erroring', async ({
+    apiRequestContext,
+    roleAccount
+  }) => {
+    const { accessToken } = await roleAccount('GROUP_MANAGER');
+
+    const response = await apiRequestContext.get('/api/v1/instruments/info?subjectId=any-subject', {
+      headers: { Authorization: `Bearer ${accessToken}` }
+    });
+
+    expect(response.status()).toBe(200);
+    expect(await response.json()).toStrictEqual([]);
   });
 });