[joshunrau]

Four fixes that were planned as separate PRs, combined into one, plus a fix for a flaky gateway test. Record edits and repository re-adds now respect the group scope of the caller's grant, file instruments can no longer be assigned remotely, and instrument lists no longer read every stored bundle.

Closes #1633, closes #1708, closes #1728, closes #1537, closes #1897

Changes

• Record edits (Instrument record PATCH is scoped by delete rules instead of update rules #1633). PATCH /v1/instrument-records/:id looks the record up and writes it scoped by the caller's update InstrumentRecord rules. It used to look the record up unscoped and scope the write by delete rules. A user granted only update can now edit records within that grant, where every request used to fail with a 500. A user whose update and delete grants name different groups now edits in the groups the update grant names. A record outside the caller's scope answers 404 before the payload is validated, so a 400 no longer reveals that the id exists.
• Remote file assignments (File instruments can be assigned remotely, but the patient's files are discarded and the record stays pending forever #1708). POST /v1/assignments, POST /v1/assignments/bulk and POST /v1/assignments/bulk/preflight refuse a FILE instrument, and a series containing one, with 422 INSTRUMENT_UNAVAILABLE. The gateway cannot deliver a patient's files, so such an assignment used to end as COMPLETE with a record stuck at pending: true. Without storage, it failed to sync on every pass instead. The Remote Assignment page and the bulk wizard no longer offer file instruments. In-person sessions are unchanged.
• Repository re-adds (Instrument repo sync and re-add ignore a group-scoped grant, letting it re-sync or replace the token of any repository #1728). Re-adding a registered repository through POST /v1/instrument-repos, which replaces its token and re-syncs it, now requires update on that repository. Anyone else gets a 409 before the token is touched. A group-scoped create InstrumentRepo grant could previously retoken a private repository used only by another group.
• Token refresh on re-add. A re-add with a new token now syncs with that token. Since c3c5dd3 (unreleased), the sync it started read the record from before the update, so the first sync still used the expired token and failed with a 502.
• Instrument reads (Instrument queries read every bundle from MongoDB even when the evaluated instance is cached #1537). InstrumentsService.find selects only ids, then reads bundle just for the instruments not already evaluated and cached. /v1/instruments/info, /v1/instruments/list and the dashboard no longer load every bundle on the instance on every request. The remote-assignability check from File instruments can be assigned remotely, but the patient's files are discarded and the record stays pending forever #1708 reads bundles the same way.
• Gateway test flake (Gateway stack-trace test fails intermittently under the full unit suite #1897). The gateway's server and router tests bind 127.0.0.1 and fetch there. They used to bind the wildcard address, which another process (the API tests' mongod) could shadow on 127.0.0.1 for the same port and answer instead.

Decisions

• Instrument queries read every bundle from MongoDB even when the evaluated instance is cached #1537 keeps fix: count every instrument edition on the dashboard #1893's count. Instrument queries read every bundle from MongoDB even when the evaluated instance is cached #1537 originally also scoped the dashboard's instrument count and made it a plain row count. fix: count every instrument edition on the dashboard #1893 has since merged a count that counts the latest edition of each instrument the group can administer, which also resolves the edition mismatch Instrument queries read every bundle from MongoDB even when the evaluated instance is cached #1537 left open. That commit is dropped here, and find's filter is no longer split into a helper.
• Instrument repo sync and re-add ignore a group-scoped grant, letting it re-sync or replace the token of any repository #1728 has no e2e test. Registering a repository in e2e would mean stubbing GitHub inside the e2e API for the whole suite. The integration suite apps/api/test/suites/06-instrument-repo-readd.suite.ts covers it server-side, with real users, real scoped grants and the stored ciphertext read back.
• File instruments can be assigned remotely, but the patient's files are discarded and the record stays pending forever #1708's new queries are unscoped by ability. InstrumentsService.findUnassignableIds only narrows ids the caller already named. It reads only archivedAt and id, and evaluates a bundle only for a non-archived instrument that isn't cached yet.

Risks

• Access changes for custom grants. Admins (manage all) are unaffected, and no base level below admin holds update or delete on records. Only users with custom permissions or a permission set are affected:

  • holders of update InstrumentRecord gain access within that grant and lose access in groups only their delete grant names;
  • holders of a group-scoped create InstrumentRepo grant can no longer re-add, retoken or re-sync another group's repository.

  Admins should review such grants before upgrading. Whether changes like these count as breaking is undecided (Release and support policy: what a version promises, which lines get fixes, and how images and npm map onto it #1870), so the commit bodies describe them in prose, with no footer.

• Existing remote file assignments. These are left in place. They can never deliver their files, so administrators should cancel any still outstanding from the subject's Assignments tab. On a storage-disabled instance, each submitted one logs a sync failure until it is cancelled.

Adversarial review

Reviewed at commit 851e740. pnpm check: green.

Findings

1. apps/api/test/suites/06-instrument-repo-readd.suite.ts:161 — the administrator re-add test, and the own-group one at :148, pass against the pre-fix code that synced with the old token: the GitHub stub answers any Authorization, and the tests check only that a download happened and the token was stored — assert the download's Authorization is Bearer new-token in both.
2. apps/api/src/instruments/instruments.service.ts:438 — findUnassignableIds and collectsFiles read every bundle even when the instance is cached, the pattern this branch removes from find — share one cache-aware id-to-instance helper, and select archivedAt and id only.
3. apps/api/src/assignments/assignments.service.ts:292 — resolveBulkRequest asks findUnassignableIds about instruments the group has already refused, evaluating bundles for nothing (a broken one would turn the 422 into a 500), and a unit test pins that call — pass only opted-in ids and test the returned issues instead.

Fixed

• 1. expectSyncedWithNewToken asserts the download's Authorization header in both tests. Both fail against syncRepo(existing).
• 2. A private findInstancesByIds is used by find, collectsFiles and findUnassignableIds, which now selects archivedAt and id and evaluates only the non-archived ids.
• 3. Only opted-in ids are passed. The test now checks that a refused instrument whose evaluation would throw still gets INSTRUMENT_UNAVAILABLE.

Attestation

I, Claude Opus 5.5, attest that the review block above reports the adversarial review as it ran on this branch at 5f30825, which the commit named differs from only by the fixes listed above, with every finding it returned.

🤖 Generated with Claude Code