fix(instrument-repos): import every instrument a repository provides

[thomasbeaudry]

Fixes three defects that made instrument repository import report an incomplete
and internally inconsistent set of instruments.

Observed on DouglasNeuroInformatics/ODC_Restricted_Instruments: 9 form
directories on GitHub, 8 in the repo list column, 4 in the "View
instruments" dialog. Three different numbers, three different causes.

---

1. Series and file instruments were never imported

discoverInstrumentDirs scanned a hardcoded list:

for (const category of ['forms', 'interactive']) {

lib/series/ and lib/file/ were skipped before discovery. Nothing appeared in
the logs because nothing was attempted — the instruments were never bundled, so
there was no error to report. Both are real kinds ($InstrumentKind is
['FILE', 'FORM', 'INTERACTIVE', 'SERIES']) with a directory in the built-in
library, so a repo laid out the same way as packages/instrument-library silently
lost half its categories.

Ordering is now load-bearing

Series carry a constraint scalars do not. validateSeriesInstrument resolves
every item to a stored instrument and rejects the whole series if any is missing:

Cannot find instrument '<name>' with edition '<edition>'

The category list is ordered so series is scanned last, and the doc comment
states that callers must import in the returned order. importInstruments
already iterates sequentially and awaits each create, so the scalars a repo
provides exist by the time its series are reached.

Known limitation, deliberately not fixed here

A series referencing an instrument from a different repository still fails,
because nothing resolves dependencies across repos or retries afterwards —
whether it works depends on the order repos were added. Making that reliable
needs a second resolution pass once every repo is imported, which is a design
change rather than a bug fix. It is now documented in
.agents/docs/architecture/instrument-pipeline.md instead of being half-built.

---

2. A lost insert race silently dropped instruments

Failed to import instrument from GAD_7: PrismaClientKnownRequestError:
Unique constraint failed on the constraint: `_id_`

Seen for six instruments, all of them names provided by more than one repository.

create guards with instrumentModel.exists({ id }) and then inserts — not
atomic. Two concurrent imports both clear the guard, and the loser gets a
driver-level unique-constraint error rather than the ConflictException that
importInstrumentFromDir knows how to recover an existing id from. So instead of
being associated with the repo that provides it, the instrument was dropped from
that repo's instrumentIds entirely.

The insert now re-checks existence when it fails and reports the same conflict:

} catch (err) {
  if (await this.instrumentModel.exists({ id })) {
    throw this.instrumentExistsConflict(id);
  }
  throw err;
}

This is exact rather than heuristic — the insert failed and the row is now
present means someone else won the race — and it avoids coupling the service to
Prisma error codes. Any other failure still propagates unchanged.

The conflict message now has a single source (instrumentExistsConflict), since
InstrumentReposService parses the id back out of it and the two throw sites
must not drift.

A genuinely silent path is now loud

If that id regex ever fails to match, importInstrumentFromDir returned null
and importInstruments did if (!result) continue; with no log above debug —
an instrument vanishing from a repo's list with no trace at default log level.
That branch now logs an error.

---

3. The dialog hid all but the latest edition

The repo table counts row.instrumentIds.length — the true stored count — while
the dialog was populated by useInstrumentInfoQuery() with no arguments. That
defaults allEditions to false, and findInfo then keys results by instrument
name, keeping only the highest edition:

const currentEntry = results.get(info.internal.name);
if (!currentEntry || !('internal' in currentEntry) || info.internal.edition > currentEntry.internal.edition) {
  results.set(info.internal.name, info);
}

8 instruments at two editions each collapse to 4 rows. Nothing was missing from
the database — the dialog wasn't asking for it. A page titled "Instruments in
<repo>", whose purpose is auditing what a repository contributed, now requests
every edition. This affects only the dialog; the table count is unchanged.

---

Tests

discoverInstrumentDirs had no coverage at all. It now has three tests: all four
categories are discovered, series comes after every scalar, and a directory
without an index file is ignored. create gains two: a lost insert race is
reported as a conflict, and an unrelated insert failure is not masked.

The two category tests and the race test were each confirmed to fail before the
fix and pass after.

Full suite: 417 passed, 1 skipped. tsc and eslint clean for apps/api and
apps/web.

Why there is no e2e test

SetupService.seedDefaultInstrumentRepo returns early when NODE_ENV=test,
because the suite must not reach out to GitHub — a constraint the existing
admin-instrument-repos.spec.ts documents in a header comment. The e2e
environment therefore contains no instrument repository and no repo-sourced
instrument, so neither the import path nor the dialog is reachable. Covering this
properly needs a mockable GitHub layer, which would unlock e2e coverage for the
whole import feature and is worth doing separately.

---

Note on merge order

This branches from main and touches instrument-repos.service.ts, which #1482
also modifies (its error-logging change is ~30 lines from this one's conflict
handling). They are separate hunks and should merge cleanly, but #1482 is the
production-breakage fix and is worth landing first.

🤖 Generated with Claude Code

[gdevenyi]

Thanks — the three root causes are well diagnosed, the discovery ordering is sound, and the race fix with its fail-first tests is solid. Two things before this lands:

1. The "View instruments" dialog can still disagree with the table's count, in exactly the case your conflict-handling fix creates. apps/web/src/routes/_app/admin/instrument-repos/index.tsx (~line 278) filters the dialog by instrument.sourceRepo?.id === repoToView.id, but provenance is only tagged on instruments an import actually created (tagInstruments(createdIds, ...)). An instrument recovered through the conflict path — the six multi-repo names from your bug #2 — joins the repo's instrumentIds, which the table counts, while sourceRepo keeps pointing at whichever repo created it first, so the dialog omits it. A manually-uploaded instrument a repo also provides behaves the same way. Filtering by membership — repoToView.instrumentIds.includes(instrument.id) — derives both the count and the list from the same field. Keep allEditions: true alongside it, since the default response drops the lower-edition ids entirely.
2. .agents/docs/architecture/instrument-pipeline.md (which this PR edits) still says the placement instruction in packages/instrument-guidelines/AGENTS.md is coupled to discoverInstrumentDirs — change one and you must change the other. This PR changes discovery to scan lib/file and lib/series, but guidelines rule 5 still only tells authors where to put forms and interactive tasks, even though the same document fully specs authoring FILE and SERIES instruments. Either extend rule 5 to name all four directories (that file ships as a published npm package, so treat the edit as a release), or add a line to the pipeline doc recording that the guidelines deliberately name only the two original directories.

The absence of an e2e test is fine as argued — the NODE_ENV=test guard in SetupService.seedDefaultInstrumentRepo and the header comment in testing/src/specs/admin-instrument-repos.spec.ts both confirm the import path is unreachable in that environment.

If you hand this to Claude Code, Opus 5 is the right size — two small but cross-workspace edits (web route, architecture doc, possibly a published package) that need repo judgment rather than pure mechanics.

Reviewed at commit ef5da95.