fix(mt#5037): Make pgvector a prerequisite in the record and in the code

[minsky-ai]

Summary

ADR-002 listed "PostgreSQL may or may not have pgvector extension available" as an environmental
constraint and decided a runtime-probing factory that returns a vector provider or a plain one.
ADR-027 re-affirmed that axis. Neither branch was reachable. The schema declares vector(1536)
columns in six tables plus six HNSW indexes, and the fresh-DB bootstrap snapshot's first statement
is CREATE EXTENSION IF NOT EXISTS vector — so a Postgres without pgvector fails at migrate with
SQLSTATE 0A000, exit 1, zero tables written, before any provider is constructed (measured on a
stock postgres:17 during mt#5016).

Two Accepted ADRs therefore documented an optionality the product does not offer. The principal
settled it via ask#11882 — "Require
pgvector" — with the cost stated and accepted: this closes off "runs on any Postgres", which
matters if a customer's managed Postgres lacks the extension.

The judgment call: the probe is converted, not deleted

The task's ## DECIDED section said to retire "the SELECT EXISTS check, the three-way outcome,
and the pgvectorVerified: false construction path" as one unit. Reading the code, those are three
different things, and the first should not be retired:

• PostgresVectorPersistenceProvider.initialize() skips its own re-probe precisely when the factory
  reports one already ran (mt#2973, postgres-provider.ts:1077). Hardcoding pgvectorVerified: true
  would therefore remove the last pgvector check on both paths — asserting a capability nothing
  verified, which is the opposite of requiring it.
• PostgresPersistenceProvider is the base class of the vector provider, not merely the degraded
  branch. "Retire the branch" can only mean the factory stops constructing it directly.
• mt#3833's inconclusive/absent distinction matters more under a hard requirement, not less:
  both now fail, so collapsing them would misreport the cause and invite the wrong remedy.

So the branch retires and the query stays, as a precondition assertion. This is an agent-level
call — it names no reserved category, the principal's decision (the ADR reconciliation direction) is
unchanged — and it is recorded in the spec as contestable.

probe outcome	before	after
present	vector provider	vector provider (unchanged)
absent	base provider, silently degraded	VectorExtensionAbsentError — not retryable
inconclusive	VectorCapabilityProbeInconclusiveError	unchanged — still retryable

Key changes

• docs/architecture/adr-002-…md — dated addendum naming the three superseded statements
  (:30 constraint, :40-42 framing, :237 "same code works in dev (no pgvector) and prod"), the
  mt#5016 evidence, what is not retired, and the decision provenance. Extends the file's existing
  addendum convention (it already carries ADR-027 2026-07-07 and ADR-035 2026-08-03).
• docs/architecture/adr-027-…md — matching addendum. Its core claim (the axis is
  intra-Postgres, not cross-backend) is unchanged and strengthened; only "supported deployment
  choice" is superseded. §Deferred's PGlite note is explicitly unaffected — PGlite is
  pgvector-capable, which is exactly what this now requires.
• vector-capability-probe.ts — new VectorExtensionAbsentError, sibling to the inconclusive
  error. The docblock states the axis that makes them two classes: retryability.
• postgres-provider-factory.ts — branch → assertion; return type narrowed to
  Promise<PostgresVectorPersistenceProvider>; the catch no longer logs a probe that answered
  as a probe that failed.
• postgres-provider.ts — pgvectorVerified's docblock now says it is a cold-boot signal, not
  a capability verdict, and why its false default is still load-bearing on the standalone path.

Testing

Execution evidence:

SC4 / SC5 / AT2 — factory asserts instead of branching, and the two errors stay distinct:

$ bun test --preload ./tests/setup.ts --timeout=15000 \
    packages/domain/src/persistence/providers/postgres-provider-factory.test.ts \
    packages/domain/src/persistence/vector-capability-probe.test.ts \
    packages/domain/src/persistence/providers/postgres-provider.test.ts
 79 pass
 0 fail
Ran 79 tests across 3 files. [1097.00ms]

SC6 / AT4 — return type narrowed; no caller depended on a degraded provider:

validate_typecheck(task: "mt#5037") → status: pass, errorCount: 0
workspaces: [".", "packages/domain", "packages/shared", "services/reviewer",
             "services/site", "src/cockpit/web", "tsconfig.hooks.json", "tsconfig.scripts.json"]
validatedWorkspace: /Users/edobry/.local/state/minsky/sessions/0ebe13c4-…
skippedProjects: [infra/tsconfig.json — deps not installed locally; CI runs it with its own install]

AT3 — negative control: the three new factory tests against the pre-fix factory

git restore --source=HEAD~1 on the factory alone, then the same test file:

(pass) a readable 'present' answer yields the vector provider
(fail) a readable 'absent' answer is now a fault — pgvector is a prerequisite
(fail) 'absent' is distinguishable from 'inconclusive' — different errors, different remedies
(fail) the probed client is ended when the extension is absent, so the pool does not leak
(pass) throws … when the probe returns zero rows
(pass) throws … a row with no exists column
(pass) throws … a row whose exists is null
(pass) throws … a row whose exists is the string 'f'
(pass) the probed client is ended when the probe is inconclusive
(pass) the string 'f' throws rather than being read as truthy
 7 pass
 3 fail

Exactly the 3 new tests fail and all 7 pre-existing ones still pass — the control discriminates
rather than breaking the suite wholesale, which is what distinguishes a faithful revert from an
inert test (mt#4502). Restored to HEAD afterwards: git status --porcelain clean, 10 pass / 0 fail.

SC1 / SC2 / SC3 / AT1 — both addenda present, citing mt#5016's measurement: in the diff;
adr-002 and adr-027 each carry a 2026-09-09, mt#5037 block naming SQLSTATE 0A000 on a stock
postgres:17.

AT1's live half is UNVERIFIED — running minsky persistence migrate against a plain
postgres:17 container was not exercised in this session. Stated rather than implied: the refusal
behaviour it asserts is mt#5016's already-merged preflight, unchanged by this PR, and the ADR half
is checkable by reading the diff. The new factory assertion is covered by unit tests above.

Deploy verification: this PR changes deploy surface (packages/domain/src/persistence/** is
application source). Post-merge I will wait on the deployment bound to this merge
(notBefore = merge time, expectCommitSha = merge SHA), read buildIdentity, and assert the
health body's service identity rather than the status code.

Planning provenance, including a gate failure

The /plan-task pass recorded gate (g) as PASS and it was a false negative — the
parallel-work guard caught PR #3412 (a 614-site repo-wide refactor) touching
postgres-provider.ts ninety seconds later. The gate's open-PR sweep narrowed 18 PRs with a
subsystem keyword filter, and a repo-wide mechanical refactor names no subsystem token by
construction. Corrected in the spec's audit with the generalizable form. Overridden via
grant-guard-override.ts on evidence: #3412 is mergeableState: dirty, carries 14 unaddressed
BLOCKING findings since 2026-09-04, and last saw a real commit on 2026-08-27 — so it is not a
bounded wait, and it must be fully rebased by whoever revives it regardless.

🤖 Generated with Claude Code

https://claude.ai/code/session_012aTeW8XXjVuEF5GykykSwq

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 583K prompt, 4K completion | Duration: 112s
Mode: normal

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

The PR consistently enforces pgvector as a prerequisite: ADR-002/027 carry dated addenda, the factory now asserts the requirement (throwing on absent/inconclusive) and no longer constructs a degraded provider, and tests cover the error paths and pool cleanup. Minor nits remain: the factory file’s top header still describes capability branching, and one test uses negative not.toBeInstanceOf assertions that could be tightened. Documentation is updated in this PR. With those small cleanups addressed in follow-ups, this is ready to merge.

Findings

• [NON-BLOCKING] packages/domain/src/persistence/providers/postgres-provider-factory.ts:1 — Top-of-file header comment still describes capability branching, which no longer matches behavior
  The file header (lines 1–5) says "Creates the appropriate PostgreSQL provider class based on runtime capabilities" while the implementation now enforces a precondition and always returns PostgresVectorPersistenceProvider (or throws). Consider updating the header summary to reflect the prerequisite/assertion design to avoid misleading future readers.
• [NON-BLOCKING] packages/domain/src/persistence/providers/postgres-provider-factory.test.ts:40 — Test only asserts negative type for error discrimination; should assert exact classes
  In the test "'absent' is distinguishable from 'inconclusive' — different errors, different remedies", the assertions use rejects.not.toBeInstanceOf(...) twice. This can pass if an unexpected error type is thrown. Prefer explicit positive checks (e.g., first toBeInstanceOf(VectorExtensionAbsentError), second toBeInstanceOf(VectorCapabilityProbeInconclusiveError)) so a regression cannot slip through with a third error type.
• [NON-BLOCKING] packages/domain/src/persistence/providers/postgres-provider-factory.test.ts:1 — Test suite title still references a "capability branch" after behavior change
  The describe block is named "PostgresProviderFactory capability branch" but the factory no longer branches by capability; it asserts a prerequisite and throws on absent/inconclusive. Consider renaming the suite title to reflect the new behavior to reduce confusion when scanning test results.

Spec verification

Criterion	Status	Evidence
SC1 — ADR-002 carries a dated amendment block stating that Postgres with pgvector is the supported deployment and names/overrides the three cited statements without rewriting history.	Met	docs/architecture/adr-002-persistence-provider-architecture.md: the 2026-09-09 addendum block is present near the top and explicitly supersedes the ## Context constraint, the ### Capability Detection Challenge framing, and the ### Operational Benefits claim; it affirms Postgres WITH pgvector as supported.
SC2 — ADR-027 carries the matching amendment against its own re-affirmation of the axis.	Met	docs/architecture/adr-027-postgres-only-persistence-confirmed.md: the 2026-09-09 addendum states the axis is a prerequisite (not a supported choice), cites mt#5016, and references ADR-002’s addendum.
SC3 — The amendments cite mt#5016's live evidence (plain postgres:17, SQLSTATE 0A000, exit 1, zero tables written).	Met	Both addenda reference the measured behavior on stock postgres:17 and SQLSTATE 0A000 (see adr-002 addendum paragraph beginning “Evidence, measured rather than asserted (mt#5016)” and adr-027’s matching paragraph).
SC4 — The factory no longer BRANCHES on the probe result; PostgresPersistenceProvider is no longer constructible via the factory.	Met	packages/domain/src/persistence/providers/postgres-provider-factory.ts:117-151 — classifyVectorProbe is used; inconclusive throws VectorCapabilityProbeInconclusiveError; absent throws VectorExtensionAbsentError; only present returns a PostgresVectorPersistenceProvider. The function’s return type is narrowed to Promise<PostgresVectorPersistenceProvider>.
SC5 — Missing pgvector yields a startup error naming the requirement and remedy, not a silently reduced-capability provider.	Met	packages/domain/src/persistence/vector-capability-probe.ts: VectorExtensionAbsentError message explicitly states the requirement and the remedy (install pgvector / use a pgvector-capable image). The factory throws this error on absent.
SC6 — pgvectorVerified is re-scoped to a cold-boot signal, not a capability verdict, and its default remains load-bearing for standalone construction.	Met	packages/domain/src/persistence/providers/postgres-provider.ts: around 578–620 — docblock updated to state it is a cold-boot optimization signal; default remains false to cover the standalone path so initialize() still checks when not factory-probed.
SC7 — pgvector-preflight.ts stays consistent (hard failure posture under the prerequisite path).	Met	packages/domain/src/persistence/providers/postgres-provider.ts: runMigrations() imports and calls assertPgvectorAvailableForMigration(this.sql) before any bootstrap/migration work, preserving the hard-fail preflight posture; no downgrade was introduced here.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
VectorExtensionAbsentError	class	packages/domain/src/persistence/providers/postgres-provider-factory.ts — thrown on probeOutcome === "absent", packages/domain/src/persistence/providers/postgres-provider-factory.test.ts — asserted via rejects.toBeInstanceOf(...)	Adopted	New error class exported from vector-capability-probe.ts; used by the factory and its tests to enforce the prerequisite behavior.

Documentation impact

• updated-in-pr — This PR updates ADR-002 and ADR-027 with dated addenda explicitly changing pgvector from an optional runtime axis to a prerequisite (see the new 2026-09-09 addendum blocks in both files). These amendments document the behavior change shipped in the factory.
  Affected: docs/architecture/adr-002-persistence-provider-architecture.md, docs/architecture/adr-027-postgres-only-persistence-confirmed.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

All revised success criteria are satisfied: ADR-002 and ADR-027 now carry dated addenda that supersede the optional-pgvector framing with measured evidence; the factory was converted from branching to a precondition assertion with clear, distinct error paths; pgvectorVerified is correctly narrowed to a cold-boot probe-skip signal; and the migration preflight remains appropriately hard-fail for explicit unavailability and fail-open for inconclusive probes. I verified the updated factory, probe module, provider classes, and ADRs. I found no new critical defects. Two non-blocking suggestions: consider logging the explicit probe outcome (absent vs inconclusive) in the factory’s answered-path log, and front-load a concise one-line summary in VectorExtensionAbsentError to avoid CLI truncation. Otherwise, this is ready to merge.

Findings

• [NON-BLOCKING] packages/domain/src/persistence/providers/postgres-provider-factory.ts:117 — Log message clarity when probe answered
  When throwing VectorExtensionAbsentError or VectorCapabilityProbeInconclusiveError, the catch distinguishes answered vs failed probes and logs either “PostgreSQL pgvector precondition not satisfied:” or “Failed to test PostgreSQL capabilities:”. Consider including the classified outcome (absent vs inconclusive) in the answered-path log to aid operators without reading the thrown error class name in structured logs.
• [NON-BLOCKING] packages/domain/src/persistence/vector-capability-probe.ts:57 — Error message length vs CLI single-line truncation
  VectorExtensionAbsentError carries a long multi-sentence message. Unlike pgvector-preflight (which provides a separate single-line summary to survive CLI truncation), this error will be rendered via the generic error handler’s first-line-only rule in some contexts. Consider front-loading a concise one-line summary before details, or mirroring the preflight’s summary/detail split for consistency.

Spec verification

Criterion	Status	Evidence
SC1 — ADR-002 carries a dated amendment block stating that Postgres with pgvector is the supported deployment… and the amendment extends the file's existing convention rather than rewriting the accepted decision text.	Met	docs/architecture/adr-002-persistence-provider-architecture.md: Addendum (2026-09-09, mt#5037) explicitly supersedes the optionality (lines near the top) and cites specific superseded statements. The file already had earlier addenda; this adds a new one without rewriting the decision.
SC2 — ADR-027 carries the matching amendment against its own re-affirmation of the axis.	Met	docs/architecture/adr-027-postgres-only-persistence-confirmed.md: Addendum (2026-09-09, mt#5037) clarifies the axis is a prerequisite, not a deployment choice; references ADR-002 addendum.
SC3 — The amendments cite mt#5016's live evidence (a plain postgres:17, SQLSTATE 0A000, exit 1, zero tables written).	Met	docs/architecture/adr-002-persistence-provider-architecture.md: Addendum block cites stock postgres:17 and SQLSTATE 0A000; docs/architecture/adr-027-postgres-only-persistence-confirmed.md addendum also references the same measured evidence and mt#5016.
SC4 — The factory no longer BRANCHES on the probe result… PostgresPersistenceProvider is no longer constructible by PostgresProviderFactory.create as a degraded fallback.	Met	packages/domain/src/persistence/providers/postgres-provider-factory.ts:50-99 — classifyVectorProbe(result); inconclusive throws VectorCapabilityProbeInconclusiveError; absent throws VectorExtensionAbsentError; only present returns a PostgresVectorPersistenceProvider. Return type is Promise<PostgresVectorPersistenceProvider>.
SC5 — A Postgres reachable but missing the extension produces a startup error naming the requirement and the remedy, not a silently reduced-capability provider.	Met	packages/domain/src/persistence/vector-capability-probe.ts:39-79 defines VectorExtensionAbsentError with a detailed remedy message; packages/domain/src/persistence/providers/postgres-provider-factory.ts:85-90 throws it on absent outcome.
SC6 — pgvectorVerified's remaining role is stated in code where it is declared… All references agree with the narrowed meaning.	Met	packages/domain/src/persistence/providers/postgres-provider.ts:170-196 — docblock explicitly reframes pgvectorVerified as a cold-boot optimization signal with false default for standalone path. Factory passes { pgvectorVerified: true } only on verified path (providers/postgres-provider-factory.ts:96-102). Vector provider skips re-probe when true (providers/postgres-provider.ts:1083-1101).
SC7 — pgvector-preflight.ts stays consistent with the above; its hard failure posture under path (a) is not downgraded.	Met	packages/domain/src/persistence/pgvector-preflight.ts — module unchanged in posture: assertPgvectorAvailableForMigration throws PgvectorUnavailableError only when availability is explicitly unavailable; still fail-open on inconclusive (lines ~132-160, 193-214). This remains appropriate under “Require pgvector.”

Documentation impact

• updated-in-pr — This PR updates ADR-002 and ADR-027 with dated addenda that change the documented stance on pgvector from an optional runtime capability to a prerequisite. I verified and cited these addenda directly in the review; no other user-facing docs were checked in this round beyond the updated ADRs. The behavior change is aligned with mt#5016’s already-shipped user guidance and preflight (not re-read here).
  Affected: docs/architecture/adr-002-persistence-provider-architecture.md, docs/architecture/adr-027-postgres-only-persistence-confirmed.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Verification round: prior non-blocking suggestions stand; no blocking issues introduced by the fix. The factory now asserts pgvector as a precondition and no longer returns a degraded provider; distinct error classes for absent vs inconclusive are in place and exercised by tests. The pgvectorVerified flag’s meaning is clarified and used only as a cold-boot probe-skip signal. ADR-002 and ADR-027 carry dated addenda consistent with the code. I found only a minor editorial duplication in comments around the probe classification. Overall, the prior blocking concerns are addressed and no new critical defects are present — approve.

Findings

• [NON-BLOCKING] packages/domain/src/persistence/providers/postgres-provider-factory.ts:98 — Redundant adjacent comment blocks describing the three-outcome probe split
  Around the const probeOutcome = classifyVectorProbe(result); site, there are two nearby comment blocks that both re-explain the three-way outcome rationale (mt#3833) and how mt#5037 changes the behavior. While both are accurate, they are duplicative and could be folded into a single coherent block to reduce repetition and drift risk. This is purely editorial and does not affect behavior; consider consolidating the two blocks the next time this file is edited.

Spec verification

Criterion	Status	Evidence
SC1 — ADR-002 carries a dated amendment block stating that Postgres with pgvector is the supported deployment… and the amendment extends the file's existing convention rather than rewriting the accepted decision text.	Met	docs/architecture/adr-002-persistence-provider-architecture.md:21-56 — new “Addendum (2026-09-09, mt#5037)” block explicitly supersedes optionality statements and affirms Postgres+pgvector as supported; file preserves prior addenda format.
SC2 — ADR-027 carries the matching amendment against its own re-affirmation of the axis.	Met	docs/architecture/adr-027-postgres-only-persistence-confirmed.md:4-24 — new addendum clarifies the axis is now a prerequisite, references ADR-002 addendum and mt#5016 evidence.
SC3 — The amendments cite mt#5016's live evidence (a plain postgres:17, SQLSTATE 0A000, exit 1, zero tables written).	Met	docs/architecture/adr-002-persistence-provider-architecture.md:29-36; docs/architecture/adr-027-postgres-only-persistence-confirmed.md:11-17 — both mention stock postgres:17 and SQLSTATE 0A000 as measured in mt#5016.
SC4 — The factory no longer BRANCHES on the probe result. PostgresPersistenceProvider is no longer constructible by PostgresProviderFactory.create as a degraded fallback.	Met	packages/domain/src/persistence/providers/postgres-provider-factory.ts:74-121 — classifyVectorProbe(result) then throws on inconclusive and absent; only present returns a PostgresVectorPersistenceProvider. Return type narrowed to Promise<PostgresVectorPersistenceProvider> (line ~61).
SC5 — A Postgres reachable but missing the extension produces a startup error naming the requirement and the remedy, not a silently reduced-capability provider.	Met	packages/domain/src/persistence/vector-capability-probe.ts:90-156 — new VectorExtensionAbsentError with explicit remedy text; thrown by factory on absent (providers/postgres-provider-factory.ts:109-111).
SC6 — pgvectorVerified's remaining role is stated in code where it is declared; all references agree with the narrowed meaning.	Met	packages/domain/src/persistence/providers/postgres-provider.ts:578-595 — docblock reframes pgvectorVerified as a cold-boot probe-skip signal; factory sets it true only on verified path (providers/postgres-provider-factory.ts:116-121).
SC7 — pgvector-preflight.ts stays consistent with the above; its hard failure posture is not downgraded.	Met	packages/domain/src/persistence/pgvector-preflight.ts — file unchanged by this PR; its existing behavior (hard-fail on explicit unavailability, fail-open on inconclusive) remains consistent with “Require pgvector.”

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
VectorExtensionAbsentError	class	packages/domain/src/persistence/providers/postgres-provider-factory.ts:106-112 — thrown on classified 'absent' outcome, packages/domain/src/persistence/providers/postgres-provider-factory.ts:140-149 — used to classify answered-probe errors for logging	Adopted	New public error class exported from vector-capability-probe; used by the factory for precondition enforcement and log classification.

Documentation impact

• updated-in-pr — This PR modifies documented architecture decisions: both ADR-002 and ADR-027 receive dated addenda that switch pgvector from an optional runtime capability to a prerequisite, citing live evidence. I verified these addenda in the diff and they align with the code change (factory now asserts). No other user docs appear affected; migration preflight behavior was previously documented in mt#5016 and is unchanged here.
  Affected: docs/architecture/adr-002-persistence-provider-architecture.md, docs/architecture/adr-027-postgres-only-persistence-confirmed.md