feat(embeddings): pluggable embedding provider with a live switch and a model/dimension safety gate (#440) - #537
Merged
Merged
Conversation
Embedding compute already sat behind the `embeddingClient@1` capability, but everything below that seam was Ollama: the contract lived inside the Ollama plugin, so a second provider would have had to depend on it for types, and nothing stopped a swapped provider from writing a second model's vectors into the one cosine-similarity space the knowledge graph keeps. The contract moves to `@omadia/plugin-api` (re-exported from `@omadia/embeddings` so out-of-repo plugins built against its dist keep compiling) and gains the `modelId`/`dimensions` metadata a consumer needs to tell two vector spaces apart. A second adapter ships for the OpenAI `/v1/embeddings` wire format, with its key in the vault rather than plugin config. And knowledge-graph activation now compares the active provider against `graph_embedding_model` (migration 0030): same dimensions means NULL the vectors and let the existing backfill re-embed, different dimensions means refuse vector writes and run FTS-only rather than degrade recall silently for weeks. Ollama-only deployments are untouched: env bootstrap still seeds only that adapter, and a deployment with no provider at all still boots.
Review round 1 rejected the first cut of the embedding-provider gate. Five findings, all of which let the feature fail in exactly the mode it exists to prevent. The gate never checked the column. `readStoredVectorDimensions()` sampled existing rows, so an empty corpus returned nothing, the model was recorded and writes were allowed. A fresh install of the OpenAI adapter would have pushed 1536-float literals into the `vector(768)` column from 0005, had every write swallowed into the attempt counter, and run permanently FTS-only while /health reported embeddings=true. The gate now reads the declared width from the catalog (`pg_attribute` / `format_type` / `atttypmod`), so an empty corpus is checked exactly like a full one, and the governed columns are discovered rather than hard-coded — a future migration adding a vector column is covered. `processes.embedding` (0009) was a second cosine space nobody governed. It is written and queried by processMemoryStore with the same client, but switchModelAndClearVectors only touched graph_nodes and processes are not in BackfillableNodeType. A same-width provider swap left process recall and the write-path dedup pre-check scoring old-model vectors against new-model queries, forever. Processes are now cleared on the same switch and the backfill sweep gained a process pass. The clear ran unbounded inside activate(). One transaction NULLing every vector for the tenant, rewriting the HNSW index, with no batching, no statement timeout, and no try/catch — a large corpus stalled activation and a gate error took the knowledge graph down with it. The kernel treats knowledgeGraph as a required service, so that throw was a boot loop. Now: bounded ctid batches under a statement_timeout, capped per activation with the remainder left to the backfill, and a gate failure degrades to the safe path. Neither adapter publishes a vector size it has not confirmed. `dimensions` carried a manifest default of 1536, which bootstrap seeds into every install, so the adapter's own dimensions<=0 guard was unreachable and an operator picking text-embedding-3-large silently got a client claiming 1536. The default is gone. Known models resolve their width from a table; an unknown model requires the field; a field contradicting a known model makes the adapter refuse to publish. The Ollama adapter publishes without provider metadata for an unknown model, so the gate treats identity as unknown as it did before #440 rather than flipping an existing deployment to FTS-only on upgrade. The FTS-only claim was too broad. contextRetriever, inconsistencyDetector, mergeCandidateDetector and topicDetector resolve embeddingClient from the service registry themselves, so the gate does not reach them. The changelog now says what is governed and what is not instead of overclaiming. Verified with the full gate: npm ci && build && lint && typecheck && test — 4878 tests, 4874 pass, 0 fail, 0 lint errors. The pg-gated suites on 127.0.0.1:55438 were unreachable during the run and stayed skipped.
…currency Two independent reviews found eight blocking issues, all in the gate's failure modes rather than its happy path. - /health read the plugin registry alone, so a boot where the gate refused every vector write still reported embeddings/semanticRecall/durableTier/ processReuse as true with no warnings. The KG plugin now publishes its gate outcome as an `embeddingModelGateStatus` service and the health snapshot reads it, naming the active model against the recorded one. - Vector writes are now refused while `clear_pending` is TRUE. Writes used to stay live during a same-width switch, so the resumed clear (which selects on `embedding IS NOT NULL` with no discriminator) destroyed fresh new-model vectors, and sustained ingest could stop the clear from draining. The invariant "a non-NULL governed vector is an old-model vector" now holds by construction. The backfill sweep is still armed so the clear finishes. - The `match` path consults `clear_pending` and resumes the clear. The switch flips the registry before clearing, so an interrupted switch matches on the next boot; the only other resumer (the backfill) is conditional. - The `embedding_attempts = 0` reset moved to its own bounded statement over `embedding IS NULL AND embedding_attempts > 0`. Riding along with the vector clear it could never match an exhausted row by construction. - The process sweep's poison filter moved into SQL (`id <> ALL($3)`); running it after LIMIT let `batchSize` failing rows starve every healthy row. - `INSERT ... ON CONFLICT DO NOTHING` now checks `RETURNING`; a lost race used to report success with this instance's own model. A disagreeing loser is blocked with the new `registry-conflict` reason. - Clear termination is sound: `FOR UPDATE SKIP LOCKED`, loop until a batch changes nothing, residual probe before declaring completion, and a session advisory lock so two clearers cannot lower each other's flag. - The registry flip runs under `pg_advisory_xact_lock` with a CAS predicate, plus a cooldown that refuses a destructive switch moments after another write on a live corpus (rolling-deploy oscillation guard). The clear machinery moved to `staleVectorClear.ts`; `embeddingModelGate.ts` re-exports it, so no import path changed. Adds `middleware/test/embeddingModelGate.pg.test.ts`, which exercises the SQL against a real Postgres + pgvector (catalog width read, the ON CONFLICT race, switch -> capped clear -> resume, advisory-lock exclusion) and self-skips when no database is reachable.
…te status live
Round 3 made the gate refuse vector writes while `clear_pending` is TRUE,
which made "a non-NULL vector is an old-model vector" true by construction —
and introduced two ways to get permanently stuck.
R1 — a throw from the clear disarmed the only resumer. Both
`clearStaleVectors` call sites in `embeddingModelGate.ts` were unwrapped, so a
per-batch `statement_timeout` (or a transient `pool.connect()` failure)
escaped to `plugin.ts`, which substituted `{status:'blocked'}`.
`requiresStaleVectorClearResume()` is false for `blocked`, so `backfillClient`
stayed undefined, the sweep was never armed, `clear_pending` stayed TRUE and
every later boot reproduced the state. Both sites now go through
`resumeClear()`, which degrades a failed clear to `pending: true` — "still
owed, resumer armed" — never to "blocked with no resumer". Worst on the switch
path, where the flip has already COMMITted with the new model name.
R2 — `unknown-provider` short-circuited before the registry was read. An
adapter carrying no metadata (pre-#440 or third-party) skipped the
`clear_pending` check entirely, so writes were allowed: the sweep cleared
every tick, the hot path refilled every tick, the flag never dropped and
/health reported `embeddings: true` throughout. The path now reads the
registry, resumes the owed clear and refuses writes until it drains.
`resumeStaleVectorClear` stays hardcoded on in the plugin on purpose — the
sweep re-checks the flag per tick, which is what covers another instance
raising it mid-run.
R3 — the published gate status was a frozen activation-time snapshot, so
/health kept reporting `stale-vector-clear-pending` until the next restart.
The status is now published through `gateStatusPublication.ts` as one stable
object over a snapshot that is replaced wholesale; the backfill calls
`onStaleVectorClearComplete` the tick the clear drains. It reports the clear
as finished, NOT vector writes as re-enabled: this process built its stores
without an embedding client, so the hot path really is off until a restart,
and `kgHealth` words that case separately.
R4 — `hasStoredVectors` probed `graph_nodes` only, so a tenant whose vectors
live in `processes` (or whose `graph_nodes` a partial clear already drained)
bypassed the anti-oscillation cooldown and took the destructive switch. It now
spans every entry of `CLEARABLE_COLUMNS`.
R5 — a connection whose ROLLBACK also failed went back to the pool inside an
aborted transaction, taking the session-scoped clear lock with it. It is now
destroyed via `client.release(true)`.
Tests: failed clear on match/switch/unknown-provider paths keeps the resumer
armed; `unknown-provider` + `clear_pending`; the gate status following the
clear to completion end to end into `buildKgHealth`; the backfill completion
callback (fires once drained, silent while owed, survives a throwing
listener); and against real Postgres — advisory lock released after both a
successful and a failed clear, the failed-clear gate outcome, the
unidentifiable-provider refusal, and the processes-only cooldown.
…ing-provider # Conflicts: # docs/CHANGELOG.md
…vector columns Two limitations of the #440 model/dimension gate, both of which forced an operator restart or a hand-written migration. GAP 3 — a gated boot could never embed again. NeonKnowledgeGraph and NeonProcessMemoryStore captured `embeddingClient` in their constructors, and plugin.ts passed `undefined` whenever the gate refused vector writes. Once the stale-vector clear that caused the refusal had drained there was no way back into the hot path short of restarting the process. Both stores now take a `resolveEmbeddingClient` resolver and call it at the moment of use; plugin.ts passes one that reads the LIVE gate status. `markStaleVectorClearComplete()` therefore re-enables writes in-process, and the published status flips `vectorWritesAllowed` back to true — which it deliberately refused to do before, because the stores genuinely could not recover. `/health` wording follows: the "restart it to re-enable them" note became wrong. A resolver returning `undefined` is byte-for-byte the old absent client: skip, no log, no attempt-counter burn. Only a client that THROWS counts as a failed attempt, and the backfill's retry cap depends on that distinction. GAP 2 — a declared-width mismatch was terminal. The KG columns are vector(768) and every OpenAI model is 1536/3072, so it is the normal case for anyone switching provider. The gate now rewrites the governed columns at the active width: capture `pg_get_indexdef` (the HNSW opclass does not encode the dimension, so replaying the captured DDL verbatim is correct and keeps the WITH/partial-predicate details hand-built SQL loses), drop indexes, drop column, re-add at the new width, recreate the indexes, reset the exhausted `embedding_attempts`, then flip the registry with `clear_pending = FALSE` — the column is empty by construction, so any owed clear is subsumed. It is destructive, so: `auto_migrate_vector_columns` (default ON), WARN-level logging of the columns, the widths and the discarded count, a `vector-columns-migrated` reason on the gate status and a matching /health warning, one transaction per table (fully old or fully new, never half), and the SAME guards the same-width switch already had — a session advisory lock in the registry namespace, the anti-oscillation cooldown, and a CAS registry flip. Bounded by a 5s budget against the 10s activate() cap; every failure path degrades to the historical `blocked` outcome with the registry untouched.
The #440 gate work made a provider swap survivable in-process: the knowledge-graph stores resolve their embedding client live and the gate auto-migrates the vector columns when the width changes. Nothing exposed that to an operator, so a switch still meant editing plugin config and restarting. Backend — new admin router at /api/v1/admin/embedding-provider (cookie session auth, mounted like its adminProviders/adminSettings siblings): GET / every installed embeddingClient@1 provider and which is active, the active model + width, the recorded corpus identity (graph_embedding_model), the LIVE gate verdict, auto_migrate_vector_columns, the governed vector columns with their stored counts, and per-candidate preview of what switching would cost. POST /switch deactivate the current provider, activate the target, re-activate the knowledge-graph so the gate re-runs. No process restart anywhere in this path. The switch validates the target against the installed providers that actually declare the capability, refuses without confirmDiscardVectors so a stray POST cannot wipe a corpus, and rolls back — restoring the previous provider and re-gating — both when activation throws and when the target activates but publishes no client (unconfigured adapter, e.g. no API key in the vault). Registry status is flipped alongside the runtime call: leaving both entries active would crash the next boot with two embeddingClient@1 providers. UI — /admin/embedding-provider, modelled on /admin/memory-backend but without its persist-and-restart shape. States the discard cost before the action, requires an explicit confirmation, polls so the live vectorWritesAllowed false→true flip shows without a reload, and renders vector-columns-migrated / stale-vector-clear-complete as in-progress information rather than as errors — both arrive with writes ALLOWED. Also exports countVectors from @omadia/knowledge-graph-neon so the router prices a switch with the same probe the migration uses, and adds the adminEmbeddingProvider namespace to both en.json and de.json.
The embedding-provider route tests fetched through the process-global undici dispatcher, which keeps sockets alive past `server.close()`. That is enough to write a request onto a socket whose server is already gone — observed once as `UND_ERR_SOCKET: other side closed` on "lets no route escape the guard applied at mount". Each harness now owns an `Agent`, destroys it in `close()`, and forces `closeAllConnections()` before waiting on the listener. The pg migration test probed Postgres at module scope with `end()` inside the `try`, so the unreachable-PG path (the normal one in CI) leaked the pool and its connect timer for the lifetime of the test process. Moved to `finally`. No behaviour change to the assertions; victims of the suite's pre-existing load-induced flakiness are untouched.
…ration
F1 (vectorColumnCatalog.ts) — captureIndexDefs joined pg_attribute only
through unnest(indkey), so an index referencing the column ONLY in its
partial predicate or in an expression was never captured. DROP COLUMN
auto-drops those anyway, so they were destroyed and never replayed while the
migration returned ok:true. Live in the shipped schema: the 0006 and 0022
backfill scan indexes are keyed on (embedding_attempts, id) and mention
embedding only in their WHERE clause — the migration silently destroyed the
indexes for the re-embedding sweep it triggers. Capture now goes through
pg_depend, the same dependency graph DROP COLUMN itself walks, so it cannot
diverge from what the drop destroys. The known limits of the capture
(constraint-backed indexes, views, triggers) are stated in the module header.
F2 (vectorColumnMigration.ts) — the attempt-reset UPDATE ran unbounded inside
the DDL transaction under a 4s statement_timeout. After the swap its predicate
degenerates to "every row that ever failed an embed", so a large tenant timed
out, rolled the whole column swap back and returned ddl-failed → blocked,
identically on every restart. It is now capped (default 5000, the same ceiling
clearStaleVectors uses) and the remainder rides on clear_pending, which arms
the two existing bounded resumers. Because the transaction already holds
AccessExclusiveLock, a short batch proves the predicate is drained.
F3 (vectorColumnMigration.ts) — the session advisory lock leaked on every
throw except one: releaseRegistryLock swallowed its own failure and the
connection went back to the pool possibly still holding it. A leaked lock in
this namespace HANGS decideRegistry's blocking pg_advisory_xact_lock. The
unlock now reports, and a connection that cannot provably release is destroyed.
F4 (vectorColumnMigration.ts) — the anti-oscillation cooldown required "and
the corpus still holds vectors", probing columns the previous migration had
just re-created empty, so it could not survive the operation it guards. It is
now armed by registry write recency alone.
F5 (adminEmbeddingProvider.ts, index.ts) — the destructive-confirmation gate
priced GRAPH_TENANT_ID while the plugin uses graph_tenant_id from its setup
form, so a configured tenant read as an empty corpus and a same-width switch
proceeded unconfirmed. The route now resolves the tenant the plugin uses and
reports it. The stale "read at the same place" comment in index.ts is corrected.
F6 (adminEmbeddingProvider.ts) — concurrent switches could leave two providers
at status 'active', crashing the next boot in activateAllInstalled. Switches
are serialised; restorePrevious verifies its post-condition and reports which
provider (if any) is actually live again.
F7 (adminEmbeddingProvider.ts) — installService.reactivate never rejects, so a
knowledge graph that failed to come back was reported as { ok: true, gate: null }.
The re-gate is now verified against the live service, and restoredProviderId is
only attached where something was restored.
Also: a migration that moved columns without flipping the registry dead-ends on
blocked/dimension-mismatch. The flip now accepts a lost CAS that already wrote
our identity, and the unrecoverable case is named in the log and on /health
instead of showing a width complaint that no longer describes the schema.
Two design calls the maintainer made on the #440 follow-up review. DECISION 1 — the provider switch must not reactivate the knowledge graph. `adminEmbeddingProvider.ts` re-ran the model/dimension gate by calling `installService.reactivate(KG_NEON)`, which runs the plugin's `close()`, which calls `graphPool.end()`. The kernel captures that pool ONCE (src/index.ts) and ~40 subsystems hold the reference — routines, dev-platform webhooks, agent schedules, cost telemetry, MCP audit, AgentGraphStore, McpConfigService. After every SUCCESSFUL switch all of them answered "Cannot use a pool after calling end on the pool" until the process was restarted, so the one feature whose selling point is "no restart" forced one. The switch now leaves the plugin up and asks the gate to re-evaluate itself. New `gateReevaluation.ts` owns both gate evaluations and publishes a `reevaluate` entry point on the `embeddingModelGateStatus` service (non-enumerable, so it never reaches the health JSON): it re-resolves the embedding client from the registry, re-runs the gate, `republish`es the verdict onto the same object the kernel holds, and re-arms or stands down the backfill sweep. Nothing is torn down. The subtle half is which client actually embeds. `plugin.ts` resolved `embeddingClient` once at activation and its resolver closed over that reference, so swapping the provider plugin underneath left the registry holding the new client while the graph kept embedding with the old one — silently, because nothing throws. The runner now owns `approvedClient()`, the client the CURRENT verdict was computed against, and a re-evaluation swaps it in ahead of publishing the new verdict. It is deliberately not "read the registry on every embed": that would let a provider activated outside this path write unvetted vectors against a verdict describing a different model. F7 machinery: `getKnowledgeGraph`/`reactivate` are gone from the router deps — with no reactivation there is no "did it come back" to check. The truthful-reporting property they carried is kept at the same strength on the state that can still fail: `KnowledgeGraphDown` is repurposed as `GateReevaluationFailed` (500, `embeddingProvider.gate_reevaluation_failed`, never `ok: true`), and a deployment with no re-evaluate entry point at all is named via `gateReevaluated: false` + `gateWarning` rather than implied. DECISION 2 — auto-migration never fires on the boot path. The destructive column rewrite was default-ON inside `activate()`. A deployment sitting on the documented `blocked/column-width-mismatch` (768-wide columns, a 1536-wide provider) that merely upgraded and restarted lost its entire embedding corpus with no prompt — `confirmDiscardVectors` existed only on the HTTP route. `autoMigrateVectorColumns` becomes `allowDestructiveColumnMigration`, a capability the caller hands to one evaluation, defaulting to FALSE. Activation does not pass it, so a width mismatch stays `blocked/column-width-mismatch`: reversible, nothing dropped, operator decides — the pre-branch behaviour. Only the re-evaluate path invoked by a confirmed switch passes it. `auto_migrate_vector_columns` is therefore a master switch over the confirmed path: 'false' forbids the rewrite even from the admin UI; 'true' permits it only when an operator confirms, and can no longer let a restart wipe a corpus. Manifest help, /health warning, changelog and both UI locales say exactly that. Inverted rather than deleted: the /health assertion that the columns were "migrated automatically" (true only while the boot path migrated) now asserts the honest wording, and the gate unit case that had to opt OUT of a default-ON rewrite documents why the explicit false is now a statement of intent. Tests: new `embeddingGateReevaluation.pg.test.ts` (real Postgres, own schema and own tenant for the hashtext advisory lock) proves a switch changes which client embeds, that the pool is never ended, that the backfill is re-armed with the new client and stood down on a block, that a width mismatch on the boot path blocks and drops nothing with the flag either way, that a confirmed switch migrates, and that the master switch still forbids it. Plus republish / re-armed-clear-hook / non-enumerable-entry-point cases and a boot-path unit case at the gate seam.
`EmbeddingBackfillHandle.stop()` only clears timers. A `runSweep()` already
executing keeps its own captured client and finishes its batch, one
`await embed()` per row followed by
`UPDATE … SET embedding = $1::vector, embedding_attempts = 0`. A same-width
provider switch runs its whole clear/republish/re-arm sequence inside that
window, so the previous provider's vectors land after the clear drained —
`clear_pending` is FALSE so nothing clears them, `embedding IS NOT NULL` so no
`WHERE embedding IS NULL` sweep revisits them, and `/health` stays green. The
invariant `clear_pending` rests on ("a non-NULL vector is an old-model vector")
is false from then on, with no path back. The hot path carries the same
resolve → await → UPDATE window.
The gate runner now owns a monotonic epoch, bumped in the same synchronous
block that swaps the approved client. Every vector writer — both backfill
passes, `embedAndStoreTurn`, `embedAndStoreNode`, `processMemory.write` and
`processMemory.edit` — captures it before its embed and drops the write if it
moved. A dropped write is a clean no-op: the row stays NULL and is not charged
an attempt, so the next sweep re-earns it with the approved client.
Two consequences of the same defect close with it:
- `markStaleVectorClearComplete` is now verdict-scoped. Guarded only on
`clearResumeOwed`, a stood-down sweep's late callback could flip writes ON
under a NEWER verdict whose own clear had not drained. Harmless while the
handler kept writes OFF; load-bearing once it turned them ON.
- a throwing `syncBackfill` no longer leaves a permitting verdict standing.
The verdict is downgraded to blocked, the epoch is bumped again, and the
error still propagates — so writes are never ON with no sweep armed while
the caller is told the switch failed.
Also surfaces `providerDrift` on Admin → Embedding provider: the registry's
live client and the governing verdict can name different models, because
swapping an adapter through the generic plugin-install UI deliberately does not
re-gate. Both numbers were already on the page; only their disagreement was
silent.
The new pg suite drives a REAL `EmbeddingBackfillHandle` through a REAL
`syncBackfill` across a live re-gate — `embeddingGateReevaluation.pg.test.ts`
substitutes a recording stub, which is why the original delta's tests missed
this. All five fence tests fail when the fence is disabled.
`NeonProcessMemoryStore.write()` was the only fenced vector writer with an `await` between its epoch check and its write: it checked before the dedup cosine probe and never again. A provider switch that completed while that query was in flight let the INSERT land a previous-provider vector in `processes.embedding` — `clear_pending` already FALSE, `embedding IS NOT NULL` so no `WHERE embedding IS NULL` sweep revisits it, `/health` green. Re-read the fence immediately before the INSERT; the pre-probe check stays, because a probe answered in the wrong cosine space can report a bogus `duplicate`. Also: - move the backfill sweep's fence capture INSIDE its try, so a throwing epoch reader cannot skip the `finally` and leave `running` TRUE for the process lifetime; correct `gateEpoch.ts`'s containment note, which claimed a guarantee three of the four writers do not have. - `describeProviderDrift` now compares vector width as well as model id (two adapters can publish the same name at different widths, and the width is what the governed `vector(n)` columns are shaped for), and no longer renders the `(gate evaluation failed: …)` sentinel as if it named a model. - the backfill reports a stale-vector clear it finds ALREADY down, under the current epoch. Reporting only from the draining tick left a liveness hole: a tick that drained the flag a second switch armed reports the wrong epoch, the report is dropped, and no later tick reports at all — `/health` kept `stale-vector-clear-pending` and vector writes stayed refused until a restart.
…ing-provider # Conflicts: # docs/CHANGELOG.md
Weegy
changed the title
Weegy
enabled auto-merge (squash)
July 30, 2026 07:02
7 tasks
Weegy
added a commit
that referenced
this pull request
Jul 30, 2026
main brought the #440/#537 embedding work, which added 10 more dev-platform references (mostly tests): 3,293 → 3,303 across src, test and packages. Third hand-edit of the baseline, same legitimate reason as the previous two — main ADDED dev-platform code; core did not re-acquire a dependency. That distinction is now stated in the README so the next raise is not read as a regression. Also fixes a sentence an earlier perl replacement broke in the README ("... It / But it counts ...") and refreshes the status section, which still claimed only Tailwind + ratchet were in flight. It now records what actually landed, what went out separately, and — more useful — the three fixes that were implemented and deliberately NOT shipped because review found real harm in them.
Weegy
added a commit
that referenced
this pull request
Jul 30, 2026
… implementation (#539) * docs(470): plugins use Tailwind, so they ship no CSS (G7 reduced) Marcel's point: web-ui is Tailwind, so require Tailwind in plugins and the missing `.css` in the ZIP allowlist stops mattering. Validated, and it is better than the workaround it replaces. The catch is the whole design: Tailwind v4 emits only classes it has SEEN. It detects them by scanning source at build time, and a plugin installed at runtime from another repository is never scanned. So this only works if core pre-generates a documented, finite vocabulary. v4 supports exactly that — `@source inline(...)` (the replacement for v3's `safelist`, brace-expandable) plus `@import "tailwindcss" source(none)` to disable scanning. Measured with the repo's own tailwindcss 4.3.3 + @tailwindcss/postcss, not estimated (probe kept as specs/470-dev-platform-plugin/ plugin-tailwind-subset.probe.css): 43,199 B raw → 7,704 B gzip → 5.7 KB brotli for layout/flex/grid/spacing/typography/borders/shadows, sm:/md:/lg: and hover:/focus:/disabled: variants, with colours restricted to the Lume tokens — .bg-accent, .text-fg-muted, .border-border, .text-danger all verified present in the output. Worth more than unblocking this extraction: - Plugins inherit the design system by construction. They get OUR colour names wired to the runtime CSS variables, so they follow the active palette and light/dark automatically and cannot hardcode a hex. - It retires a known drift hazard. middleware/src/admin-ui/ harness-admin-css.ts is 345 hand-maintained lines whose own header says "mirror web-ui/app/_lib/theme.css; keep the two roughly in sync when the design system changes". Generating both from the same tokens removes the sync obligation instead of restating it. - It is enforceable: reject `[` in class attributes at ingest. HARD CONSTRAINT now in the contract: no arbitrary values (`w-[137px]`, `bg-[#abc]`). That space is unbounded, cannot be pre-generated, and such a class renders unstyled with no diagnostic — the worst failure mode. Documentation alone is not enough; it needs the ingest check. Implementation note: the `@theme inline` bridge currently lives inside globals.css:16-48 and must be extracted to its own file that both it and the plugin stylesheet import — otherwise the two drift, which is the exact failure this is meant to end. G7 is downgraded from "hard blocker" to the JS-bundle question alone: `.js` and `.map` are already allowlisted, so a compiled SPA can ship today; what is still missing is a static-asset serving path from the plugin's router. Much smaller than a styling story. * feat(470): automated decoupling ratchet + functional acceptance matrix Answers a question the existing docs could not: how do we KNOW every function got extracted and that the result is installable? They could not, and this is the gap: - core-decoupling-checklist.md enumerates FILES. You can move all ~200 and still silently lose a feature — a file inventory cannot tell you a capability survived. - plan.md stated success criteria in prose. Prose is not a probe. - The only "completeness check" was a single `rg` in the P6 exit criterion, i.e. a one-shot grep nobody runs. Two additions. 1. scripts/check-core-decoupling.mjs — a ratchet, wired into CI as the `core decoupling ratchet (#470)` job. Counts Dev Platform references across 12 zones of core and FAILS if the count rises. Baseline is 3,171 (middleware/src 1621, test 925, web-ui/app 192, sidecars 180, packages 86, migrations 70, compose 39, scripts 30, ci 16, messages 4). `--update` only ever lowers it; raising it needs a hand-edit, so a new coupling shows up in review instead of slipping in. This is what makes the checklist's staleness survivable: even if the sweep missed a reference, the count still sees it, and the count cannot reach zero while it survives. It also stops core re-acquiring a dependency mid-extraction, which is the realistic failure mode for a multi-week epic touching ~200 files. And it turns "finished" into a machine-checked fact (count 0) rather than an assertion. Verified in both directions: passes at baseline, exits 1 with the offending zone named when a reference is added. 2. specs/470-dev-platform-plugin/acceptance.md — the functional contract, which is the actual answer to "all the functions". 34 HTTP endpoints (9 job admin, 9 repo/credential, 2 gates, 5 GitHub App, 7 runner phone-home, job-policy, webhook), 3 chat tools, ctx.devJobs, 4 background loops, 4 UI screens, the chat card, the dev-transcript CLI and the conductor `dev.job` step kind — each with an owner and a probe. Plus install/uninstall/upgrade acceptance, which does not exist yet and belongs in P4. Rows whose MECHANISM must exist in core first are marked: the seven runner endpoints and the App callback need H1 (public paths), the webhook needs G3 (raw body), the chat card needs H3, the conductor step needs H2, ctx.devJobs needs the G8 contract decision. Honest about what is still not covered, in acceptance.md §4: the capability matrix is a review checklist rather than a smoke suite, install/uninstall cannot be tested before P3/P4, and once the plugin leaves this repo nothing here verifies it still satisfies §2 — that becomes the plugin repo's CI against a published core contract. Also flagged: the boot-time safety refusals (SUBSCRIPTION_MODE without ACK, UNSAFE_LOCAL without LOCAL_UID) must become activation refusals, or misconfiguration silently activates instead of failing closed. Stacked on the Tailwind commit because both edit plan.md; the ratchet is independently reviewable and independently revertible. * docs(470): index the epic — one entry point for plan, checklist, acceptance Marcel wants the whole planning to live in ONE PR, because the implementation happens there too. This is the entry point that makes that real: what each of the four documents answers, what already merged via #536, what is in flight, and the two decisions that block code. Also writes down the working agreement for a long-lived epic PR: one commit per phase so ~49k LOC stays reviewable and revertible; wire paths frozen (deployed runners phone home to literal URLs); do not delete the publicPaths exemptions before H1 is proven; and the abandonment checkpoint after P3/P3b. * docs(470): implementation plan from six parallel design passes One design pass per hard problem (H1 public paths, H2 conductor step kinds, H3 chat card + plugin UI, G4 plugin SQL + migration handoff, G8 plugin-api contract, P4 repo split + supply chain). specs/470-dev- platform-plugin/implementation.md is the synthesis: what they changed, what they found, and the PR sequence. Five decisions in plan.md were wrong or under-specified: 1. publicPaths must NOT become a dynamic set. requireAuth runs before routing and structurally cannot know who will answer, so putting the grant there rebuilds the hole it is meant to close. Use a mount slot BEFORE requireAuth that terminates — fail-closed by construction, and publicPaths.ts stays a frozen literal. 2. The chat card is neither a generic schema nor a degradation. A generic node tree makes core a rendering engine for untrusted markup; degradation makes the human gate — the platform's principal safety mechanism — annoying, and annoying safety mechanisms get bypassed. A closed 7-node contract with liveness mediated by core. A plugin-supplied SSE URL would be an SSRF aimed at the operator's own session. 3. Do NOT add .css to the ZIP allowlist. The inability to ship CSS IS the enforcement for the Tailwind vocabulary. 4. The plugin-api break has no installed base — see below. 5. The vault re-key, not the migrations, is the most irreversible step. Migrations are idempotent and additive; a deleted GitHub App private key is gone. Six live bugs found, none caused by the extraction. Two verified here: B1 ctx.services.get is completely ungated (platform/pluginContext.ts :230-233 is a bare pass-through). Any installed plugin can call ctx.services.get('graphPool') and receive the superuser pg.Pool — full read/write on users, conductor_runs, everything. No manifest declaration, nothing in the install dialog. Biggest hole found in this epic and it is live today. B4 .sql is not in the ZIP extension allowlist, so a distributed plugin cannot ship migrations at all. Blocks G4 exactly as the missing .css blocked G7. Reported and not yet independently verified: ServiceRegistry is never disposed on deactivate (same class as the router bug fixed in #536, one layer down); all five core migrators race on multi-replica boot with no advisory lock; the conductor dev-job step is dead code in production so the reconciliation sweep has never run; dev_repo_plugin_grants is never cleaned on uninstall. And a trap in my own plan: removing ctx.devJobs without replacing its gate would have converted a permission-gated, kernel-attributed accessor into an ungated, self-attributed one — because B1 is the only remaining path. Two designs flagged it independently from different directions. Two risks moved in opposite directions. H2 got much cheaper: the conductor step never ran, so there are almost certainly no live dev_job awaits and backwards compatibility is nearly free. G8's SemVer risk evaporated: @omadia/plugin-api is private:true and its publish job is gated `if: false` — never published, no installed base, so take the break now and cut 1.0.0 clean. H3 and P4 got visibly more expensive. Also newly found: next/font and data-theme do not cross an iframe boundary (silent font and dark-mode regressions), and keyless cosign binds the certificate identity to repo+workflow+ref — publishing the same image from the new repo makes every daemon with a pinned identity refuse to launch jobs. The migration handoff needs per-file schema WITNESSES, not trust in the donor ledger. Donor rows present with tables absent — a restore, a skewed rollback, an incident — makes a naive seed activate green while every request 500s. Sequence: Phase A (C1-C8) ships five reusable platform capabilities and moves zero dev-platform code, ending at the abandonment checkpoint. Phase B (P0-P5, C10-C13) is copy → prove → delete, with the proof gate at P5 and the two publicPaths exemptions deleted last, alone, in a revertible commit. Six decisions block work; D1 (publish plugin-api to public npm) blocks everything after it. * docs(470): correct D1 — publishing plugin-api was never required Marcel pushed back on D1 ("warum ist das der Blocker? Wir brauchen das doch nicht public?!") and he was right. The evidence was two directories away and I did not look. - omadia-byte5-plugins already solves this in production for six private plugins: package.json declares "@omadia/plugin-api": "file:../odoo-bot/middleware/packages/plugin-api" with the sub-packages carrying "*" as a peer resolved by the workspace root. No registry, no publish, nothing public. - The boilerplate contract mandates the OPPOSITE of what I recommended. Point 1: "KEIN Cross-Import ... Die Interface-Definition wird bewusst in ./types.ts dupliziert ... Absicht nicht Bug." omadia-plugin-starter ships vendored types/omadia-plugin-api.d.ts for exactly this. - There is no runtime dependency at all: every @omadia/plugin-api import in the dev-platform tree is `import type` and vanishes from the emitted JS. Even a value import would resolve against the host's own node_modules, which the uploaded-package store symlinks in. Root cause of the error: the design pass recommended public npm on a PRODUCT argument — GitHub Packages needs auth even to read, which would hurt a third-party plugin ecosystem. Sound for a public ecosystem, irrelevant for a private byte5 plugin. I passed it through as a technical blocker without checking how this org actually builds private plugins. D1 drops from "blocks everything after it" to a P3-typecheck-only choice between three options, none of them public: file: sibling (proven, but the plugin repo's CI then needs a core checkout — friction already recorded in project memory), vendored .d.ts (CI-isolated, drifts silently), or a git dependency on a tag (CI-isolated, explicit version). Consequence for the sequence: the first real step is no longer C1 but the B-fix PR — the three live bugs that are wrong today independently of this epic. * fix(470): corrections from the codex deep-check Full verification pass against the code (GPT-5.6, reasoning=high) over all five planning documents. It found eleven errors. The three that would have cost the most: 1. THREE CAPABILITIES IN THE ACCEPTANCE MATRIX ARE DEAD IN PRODUCTION. Verified here: - the conductor dev.job step — conductor/index.ts builds the executor with no devJob dep, so the dispatch branch never fires - ctx.devJobs — `provide('devJobs', …)` exists NOWHERE in src/, so the accessor throws on every call - tracker polling — TrackerPoller is never constructed or started The matrix was written from source, and source presence is not production reality. It would have certified preservation of capabilities the operator never had. Marked in acceptance.md; each needs a delete-or-wire decision in P2b. 2. THE RATCHET HAD A ZONE GAP AND OVERLAPPING ZONES. middleware/.env.example (19 references) was covered by no zone, so the count could have read 0 while it still documented DEV_* keys — the exact false-negative that would make "0 means done" a lie. And a root-config zone rescanned the whole web-ui tree, double-counting web-ui/app. Fixed: 14 depth-bounded, disjoint zones; baseline 3,171 → 3,181; and the check is now PER ZONE, because an aggregate-only comparison passes while one zone falls and another rises — which is precisely what a half-finished move looks like. Verified the guard now catches a regression in the previously invisible zone. acceptance.md now states plainly what the ratchet does NOT prove: it counts identifiers, not behaviour. Necessary condition, not sufficient. The earlier "machine-checked definition of completion" claim was too strong. 3. A MISSING CAPABILITY, WHICH IS THE DANGEROUS DIRECTION. The endpoint count was 34; it is 35 business endpoints / 36 handlers. The miscount hid the omission: the LLM proxy has TWO handlers and only one was listed. GET /api/v1/dev-runner/llm/ is a liveness probe the CLI depends on and it was absent from the matrix entirely. Also corrected: - The wireDevPlatform ↔ routes "cycle" is NOT an import cycle. wireDevPlatform is imported only by index.ts and no route imports back. One-way layering inversion. C3 is boundary cleanup and must not be justified as fixing hoist-dependent behaviour. - `pgPool@1` invented a capability name; the established contract is `graphPool@1`, already provided by harness-knowledge-graph-neon. Second D1-class error — a recommendation contradicting house practice. Now: gate the EXISTING graphPool@1 behind permissions.sql. - C1 still said "publish to public npm" — residue of the corrected D1. plugin-api stays private:true; only the .d.ts golden snapshot lands. - B3: at least eight migrators race, not five. - B1: the hole is real, the "superuser" characterisation is unproven. - B6: the MCP grant bug is live; the dev-repo grant half is unwired. - Arbitrary Tailwind values CAN be pre-generated when named exactly (@source inline("w-[137px]") emits it). What cannot is the unbounded universe. The vocabulary argument holds; the absolute phrasing did not. And ingest sees compiled Vite JS, not JSX class attributes, so "reject [ in class attributes" is under-specified. - src/devplatform is 53 files / 14,457 LOC, not 54 / 14,498. - plan.md §4.1 and §4.2 contradicted each other on DevJob type ownership. §4.2 wins: the types move to the plugin repo. Flagged, not yet resolved: a Vite multi-file SPA is not supported by today's plugin contract (boilerplate mandates single-file HTML and a tsc-only build), so P2 is viable only after C8 ships static serving. And unknown manifest keys are silently IGNORED, not rejected — a plugin declaring permissions.public_paths against an unpatched core would activate with no grant and no error. * feat(470): decide the dormant capabilities — and there are five, not three Three design passes (one per capability) plus a codex verification round. The verdicts differ, which is the finding — "activate all three" would have been wrong. 1. Conductor dev.job step → ACTIVATE as its own PR (C5b), or delete 2. ctx.devJobs → DELETE 3. Tracker polling → DEFER, move dormant 4. TrackerRegistry → DEFER, moves with #3 (newly found) 5. Comment-back → DEFER, moves with #3 (newly found) #4 and #5 surfaced while designing #3. acceptance.md listed comment-back as live with the probe "result posted to the issue" — it is not wired, so a polled job's result never reaches the issue and the loop is half-open even if the poller ran. THE DECISIVE FINDING is on ctx.devJobs, and it inverts the intuition. Every access gate lives in the ACCESSOR, and every identity is a parameter the CALLER passes — listGrantedRepoIds(pluginId), cancelJob(jobId, requestedByPluginId), createdBy:{kind:'plugin',id}. Verified: the host service itself verifies nothing, and DevRepoPluginGrantStore is never constructed, so the grant table has no writer at all. Combined with the ungated ctx.services.get (B1), the moment ANYONE registers 'devJobs' — core today or the extracted plugin tomorrow — any installed plugin can fetch it with no manifest declaration and no operator consent, pass an arbitrary pluginId, and bypass the permission gate, the repo-grant scope, the creator check and the audit attribution, while framing another plugin. So the dead state is SAFER than the wired state. "It throws on every call" is currently load-bearing. That also sharpens implementation.md §2.2 by a notch: I had written that REMOVING ctx.devJobs without replacing its gate opens the hole. True — but ADDING the provider opens it too. `provide` is the dangerous operation. C2 bundling the gate fix with the removal is a correctness requirement, not a convenience. And a test-shape lesson worth more than this epic: pluginDevJobsAccessor.test.ts has a case titled "throws a clear error when the host service is unregistered" — it asserts the PRODUCTION BEHAVIOUR as the error path and stays green, against a two-line fake registry. No test in the repo boots a real ServiceRegistry and asks whether anything provided the service. A boot-level accessor/provider invariant should ship independent of #470. ONE THING SHIPS NOW: cross-source trigger dedupe. hasActiveTriggerJob filters on `source` and dev_jobs_webhook_one_active is scoped WHERE source='webhook', so a repo with both triggers would get two runners, two LLM budgets and two PRs for one issue. Latent only because no tracker job has ever been created. It is the only artifact here that is not thrown away by the extraction. CORRECTIONS FROM THE VERIFICATION ROUND — the first draft had errors, and two of them understated risk: - cold start costs $500, not $150 (default budget is $5, page limit 100). Understated by 3x. - "a dry run would spend real money" was overstated: the supported route is previewRun, which explicitly stubs action steps. - "nobody could ever author the step" is wrong — validation is bypassed on the raw POST / path. - "a poller with nothing to poll" is overstated — TrackerRegistry has a built-in GitHub fallback needing no registration. - hooking DevJobStore.finishTerminal contradicts our own contract: finalizeDevJob is the documented choke point, and the decoupling checklist names it. Fix the split finalizer wiring instead. - phase-engine terminals do NOT bypass boundFinalize; only the worker-driven ones do. - the widened index is not "mandatory today" — the existing webhook-only index already makes the live path replica-safe — and it is NOT a safe drop-in: it needs a duplicate preflight, and it should land after the migrator advisory-lock fix, not before. PROCESS FAILURE, named because it matters: the first draft proposed resolutions and did not propagate them, leaving acceptance.md and implementation.md still saying "three" and still requiring preservation of things this document deletes. A decision doc that contradicts its siblings leaves the spec set worse than before. Propagated here, along with the stale README ratchet numbers (12 zones/3,171 → 14 zones/3,181). * docs(470): record Marcel's answers — devJobs delete confirmed, tracker is a roadmap foundation Two open questions answered, and the second changes more than the first. NO customer-side or unreleased plugin declares permissions.devJobs. The DELETE verdict for ctx.devJobs is confirmed — the single fact that could have inverted it does not exist. G8 collapses almost entirely with it, and the plugin-api major bump becomes hygiene rather than a break with downstream cost. YES, a Jira/Linear tracker is on the roadmap and is considered important. The tracker verdict keeps its direction — defer, move dormant — but loses its meaning: 'and forget about it' was wrong. TrackerRegistry is not dead weight being tolerated, it is the extension point for a roadmap feature. Three consequences: - The blockers stop being hypothetical. Cold start ($500 ceiling), requireGate:false with no sender allowlist, firing on any ticket update rather than on label application, and the cross-source dedupe gap become must-fix before a Jira tracker runs. - A new architectural question, in no document until now: after extraction the registry lives in the dev-platform PLUGIN repo, so a Jira tracker would be a plugin registering into another plugin's registry. That seam constrains the extraction — it may argue for keeping a generic job-trigger-source extension point in CORE rather than moving the registry out. - It inherits B1. A tracker registry is a WRITE surface: registering a tracker influences which issues become code-execution jobs. With ctx.services.get ungated, any plugin could register one. The per-caller-factory fix in C2 becomes a prerequisite, not optional hardening. Design pass on the cross-plugin seam is in flight. * feat(470): invert the tracker seam — provider, not registry Marcel confirmed a Jira/Linear tracker IS on the roadmap and matters. That turned "move the registry dormant and forget it" into an architecture question: after extraction the registry lives in the dev-platform PLUGIN repo, so a Jira tracker would be a plugin registering into another plugin's registry. The answer is to invert the direction: Jira plugin = PROVIDER provides: ["devTracker.jira@1"] dev-platform = CONSUMER services.get('devTracker.' + repo.trackerKind) per repo, per sweep — no tracker `requires` TrackerRegistry is then DELETED, not moved. Its plugin-map half becomes the services.get lookup; its GitHub-fallback half folds into dev-platform's own resolver. The naive direction fails four ways: it hands a MUTABLE registry through an ungated accessor; registerTracker(kind, factory) has no caller attribution (identity is the key the caller picks); services.replace() is an exposed MITM primitive; and the ABI is DevRepo-shaped — a ~40-field internal type that moves to the plugin repo at P4, paired with a return type from a core route file deleted at C10. Both sides of that signature cease to exist where a third party can reach them. Inverted, the object crossing the seam is a read-only stateless service — the same risk class as graphPool@1, which this org already ships. And it is what makes C2's per-caller factory pay off: the credential owner decides who may use its credentials. Applied to a shared registry the factory would gate who may REGISTER, which is the wrong question. THREE VERIFIED FINDINGS, all with consequences beyond the tracker: 1. The hot-install path bypasses capability resolution entirely. index.ts routes `case 'extension'` straight to toolPluginRuntime.activate(agentId) — no resolveEligiblePlugins, no topo-sort. So `requires`-based ordering applies only on the BOOT path; for the normal case (operator installs from the hub at runtime) it does nothing. Any design leaning on activation ordering is already broken there — which weakens ordering arguments elsewhere in these docs, including the ctx.devJobs inversion discussion. 2. findDependents checks depends_on only, never capability `requires`. An operator can uninstall a provider with live consumers, no 409. 3. source_ref is `owner/name#N`. FINDING 3 INVERTS THE SHIP ORDER I RECOMMENDED. A Jira PROJ-123 coerced to 123 collides with GitHub issue #123, so widening the unique index BEFORE namespacing source_ref ships a false-POSITIVE dedupe: a Jira ticket silently suppressing an unrelated GitHub issue. Namespace first (jira:PROJ-123), widen second. And the dedupe fix is less load-bearing than I claimed. listPollableRepos selects `... AND (tracker_kind IS NOT NULL OR credential_kind = 'github_app')` — that OR is what drags webhook-covered GitHub repos into the poll set, the sole source of the double-job risk. Delete the built-in GitHub fallback and no repo is ever both polled and webhooked for the same ticket. The widened index drops to defence-in-depth (still worth having: migration 0025's source='plugin' is a third potential writer). Verdict changes: - Tracker polling: DEFER → DEFER-AND-HARDEN. Behind a flag, contract frozen, expiry kept. P3's exit condition becomes "cannot be switched on without all six hardening fixes". - TrackerRegistry: DEFER → DELETE. Nothing to move once inverted, and that is what lets the ratchet reach 0 without an allowlist entry. - Comment-back: no longer "moves with #3" — REWRITTEN at P3 against the tracker contract; only the marker/idempotency logic survives. The contract must be frozen BEFORE the poller is hardened: Ticket needs ticketId (opaque string), displayKey, labels[] and labelAppliedAt, plus updatedSince as a provider parameter. Without labelAppliedAt the "fires on any update" bug is unfixable at the consumer. Home: src/devplatform/trackerContract.ts in Phase A, travelling at P4 — the treatment already agreed for devJobTypes.ts. * docs(470): scope correction — conductor dev-job is delete, not genericise Marcel: 'Der Conductor ist eine neue Funktion. Was hat das mit der Dev Platform zu tun?' Correct, and it exposed a scope error. The Conductor is a real, live feature (31 files, 6202 LOC backend, 23 UI files, 7 migrations, its own spec). It is in this epic only because its code holds 73 dev-platform references that must leave for the ratchet to reach 0 — not because anything about Conductor itself is being changed. I turned that into 'build a generic step-kind registry (H2/C5) and activate the step (C5b)' — a new platform capability plus a new feature, neither of which anyone asked for, propagated through the plan as a hard blocker. Removing a dev-platform reference from core has exactly two paths: genericise, or delete. Genericising is only justified when something real needs the generic version. Nothing did. So G9 drops from hard blocker to a deletion, C5 shrinks to 'delete dead code', and C5b disappears along with the await_kind migration, the registry deactivation semantics and the cross-kind guard — all unbuilt. Deleting is not lost work: the existing code is dev-job-SHAPED, so a generic registry would replace it anyway. Only the design has value, and that survives in this document. * chore(470): resync with main (PR #529) and re-baseline the ratchet main brought PR #529 — a substantial dev-platform change: 59 files, +3,751 LOC. New web-ui surfaces (PhaseArtifactPanel, PrettyArtifact, ToolCallCard, lineDiff/prettyArtifact/toolCallLog libs + tests), LLM proxy test coverage, and 27 new i18n lines per locale. The ratchet caught it exactly as designed: 3,181 → 3,293 across six zones (src +14, test +32, packages +10, sidecars +15, web-ui/app +35, compose +6). This is the documented hand-edit case — main legitimately ADDED dev-platform code, so the count rises for a legitimate reason rather than core re-acquiring a dependency. Baseline raised deliberately and recorded here. Re-measured, since the checklist is a snapshot: src/devplatform 53 files, 14,457 → 14,520 LOC web-ui admin surface 20 files / 3,163 → 29 files / 4,344 LOC adminDevPlatform i18n 269 → 288 keys (3,205 total) Also propagated the verdicts into acceptance.md's warning box, which still carried the pre-correction versions: conductor step now DELETE (not "activate as C5b"), TrackerRegistry DELETE (the seam inverts), comment-back REWRITE at P3, polling DEFER-AND-HARDEN. Verification after merge: middleware build + typecheck clean, 5,044 pass; web-ui typecheck clean, 388 pass, i18n parity OK at 3,205 keys. One middleware failure did not reproduce on re-run — consistent with the pre-existing load-sensitive flakiness already documented (a file of 48 trivial assertions reproduces it; baseline without added files is clean). * fix(platform): dispose plugin-provided services on deactivate ServiceRegistry had no owner tracking and no disposeBySource, and toolPluginRuntime.deactivate() disposed routes and uiRoutes but not services. A provider whose close() forgets its handle left the service registered against a torn-down module, and reinstall then threw "duplicate provider". Same bug class PR #536 fixed for Express routers, one layer down. - serviceRegistry.ts: owner tracking on provide()/replace(), and disposeBySource() unwinding LIFO — an older `replace` restore would otherwise reinstate a provider a newer one has since shadowed. `owner` is optional, so core's ~25 boot-time provide() calls stay untracked and can never be bulk-disposed. - pluginContext.ts: ctx.services.provide/replace pass agentId, so attribution comes from the kernel-known id and never from a caller-supplied argument. No plugin-api contract change. - toolPluginRuntime.ts: disposeBySource before the awaited close(), same 5s-budget reasoning as the route disposal, plus the activate-failure rollback. - dynamicAgentRuntime.ts: same gap confirmed and mirrored. 13 tests. Verified fail-without-fix in three staged reverts: reverting both runtimes gives 4 real assertion failures (not TypeErrors); reverting the context fix alone fails the attribution test; full pre-fix state fails 11. Also fixes toolPluginRuntimeRouteDisposal.test.ts's fixture, which omitted the now-required serviceRegistry dep — fixed the fixture rather than making the production call defensive, since the real wiring always supplies it. 5,058 pass, typecheck and lint clean. TWO OTHER FIXES FROM THIS BATCH WERE DELIBERATELY NOT SHIPPED — see the follow-up notes. Adding '.sql' to the zip allowlist would weaponise a pre-existing path traversal into arbitrary SQL execution, and wrapping the migrators in an unbounded advisory lock would convert a rare race into a deterministic boot failure. Both verified against the code. Known gaps in this fix, both worth follow-ups: - withTimeout is a bare Promise.race and does not cancel, so a timed-out activate can still register services after the rollback ran. - DynamicAgentRuntime.activate() has no rollback block at all, and its route disposal still sits after the awaited close() — the pre-#536 ordering. * chore(470): resync with main (#537) and re-baseline; refresh status main brought the #440/#537 embedding work, which added 10 more dev-platform references (mostly tests): 3,293 → 3,303 across src, test and packages. Third hand-edit of the baseline, same legitimate reason as the previous two — main ADDED dev-platform code; core did not re-acquire a dependency. That distinction is now stated in the README so the next raise is not read as a regression. Also fixes a sentence an earlier perl replacement broke in the README ("... It / But it counts ...") and refreshes the status section, which still claimed only Tailwind + ratchet were in flight. It now records what actually landed, what went out separately, and — more useful — the three fixes that were implemented and deliberately NOT shipped because review found real harm in them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Embedding compute already sat behind the
embeddingClient@1capability, but everything below that seam was Ollama-specific: theEmbeddingClientcontract lived inside the Ollama plugin, config and env bootstrap were Ollama-named, and the knowledge graph pinnedvector(768)with no per-row model metadata — so swapping providers would silently mix incompatible vectors into one cosine space.This makes the provider pluggable, mirrors the split the LLM side already has between
llm-provider-apiand its adapters, and lets an operator switch provider from the admin UI without restarting the process.1. Contract extraction
EmbeddingClientmoved to@omadia/plugin-api, extended with anEmbeddingProvidertype carryingmodelIdanddimensions.withConcurrencyLimitandEmbeddingErrormoved too, so a second adapter needn't depend on the Ollama package.@omadia/embeddingsre-exports all of it, so out-of-repo plugins compiled against itsdist/keep working. The capability name is unchanged and no consumer manifest was touched.2. Second adapter
New
@omadia/embedding-adapter-openaiover the OpenAI wire format (POST {base_url}/v1/embeddings— OpenAI, Azure behind a gateway, vLLM, LM Studio, LiteLLM). The API key is asecret-typed field read viactx.secrets.get(), so it lives in the vault, never ininstalled.json. Because it declares a secret field the catch-all bootstrap skips auto-install, so adding a second provider stays an explicit operator act — andctx.services.providestill throws if two are ever active.Neither adapter publishes a vector size it has not confirmed.
dimensionscarries no manifest default; known models resolve their width from a table, an unknown model requires the field, and a field contradicting a known model makes the adapter refuse to publish.3. Model/dimension safety gate
Migration
0030_embedding_model_registry.sqlrecords the activemodel_id+dimensionsper tenant. On activation the gate reads the declared width of every governed vector column from the catalog (pg_attribute/format_type/atttypmod) — not sampled from rows, so an empty corpus is checked like a full one — governsprocesses.embeddingalongsidegraph_nodes.embedding, and refuses writes on a mismatch instead of letting Postgres reject each row into the attempt counter. A same-width model switch clears vectors in boundedctidbatches under astatement_timeoutand lets the backfill re-embed, refusing writes until the clear drains so "a non-NULL vector is an old-model vector" holds by construction. Switches serialise withpg_advisory_xact_lock+ a CAS predicate and a 10-minute anti-oscillation cooldown./healthreflects the gate outcome rather than just "is a plugin active" — it previously reportedembeddings: true, warnings: []while the gate blocked every write.4. Live provider switch, no restart
middleware/src/routes/adminEmbeddingProvider.ts+web-ui/app/admin/embedding-provider/. The page shows the active provider and model, the recorded corpus model and width, live gate state, and — before a destructive switch — exactly how many vectors will be discarded, behind an explicit confirmation checkbox.Two mechanisms make it safe:
close()ends thegraphPoolthe kernel captures once as a const and ~40 subsystems hold, so reactivating it would break routines, dev-platform webhooks, agent schedules, cost telemetry and MCP audit until a process restart.await embed()and re-reads it immediately before the write, skipping the write if it moved. Without this, an in-flight backfill tick or a slow hot-path embed lands a previous-provider vector after the switch already cleared the corpus — leaving rows that are non-NULL (so no clear revisits them) and not-NULL-predicated (so no sweep revisits them), permanently, with/healthgreen.5. Runtime column migration — operator-initiated only
On a width mismatch the gate can rewrite every governed vector column to the provider's width: capture index definitions via
pg_depend(the same dependency graphDROP COLUMNwalks, so the capture cannot diverge from what the drop destroys), drop indexes, drop and re-add the column, replay the indexes, reset attempt counters, flip the registry.This never fires on the boot path. A restart cannot destroy a corpus.
allowDestructiveColumnMigrationdefaults to false and activation never requests it; only aconfirmDiscardVectorsswitch through the UI does, andauto_migrate_vector_columnsis a master switch an operator can use to forbid the rewrite even then.Verification
Full gate green on both packages: middleware
npm ci && build && lint && typecheck && test, and web-uilint && typecheck && test.Four pg-backed suites exercise the SQL against real Postgres + pgvector — catalog width reads, the
ON CONFLICTrace, concurrent gates, switch → capped clear → advisory-lock exclusion → resume, cooldown refusal, advisory-lock release after success and after failure, index capture and byte-identical replay of the shipped partial indexes from0006/0022, and the write fence under a genuinely in-flight sweep. They self-skip in CI, because themiddleware (lint + typecheck + test)job declares no postgres service — the pgvector service belongs to the separateschemajob. Wiring one up is the single highest-value follow-up here: nearly everything worth testing in this change is concurrency and catalog behaviour a mocked pool cannot reach.The
atttypmodwidth extraction was verified empirically rather than assumed:vector(768)→768(pgvector stores the raw dimension, unlikevarchar's n+4), untypedvector→-1→ skipped, andvector(n)[]hastypname = '_vector'so arrays are excluded.Known limitations — please read before merging
embeddingClient@1adapter through the generic plugin-install UI leaves the knowledge graph embedding with the previously approved client until a restart or a real switch. This is deliberate: resolving live from the registry on every embed would let an unvetted provider write into a governed cosine space against a verdict computed for a different model. The admin page now raises aproviderDriftwarning when the registry's client and the gate's verdict name different models.matchstate is not detectable.match/re-embeddingpublish a baremodelIdand the published status carries no numeric width, so drift detection only coversblockedandcolumn-migrated. Closing it needs a new field on the published status.promoteDurableRule.tsbypasses the gate entirely.harness-orchestrator-extrasembeds and writesgraph_nodes.embeddingwith the raw registry client captured at activation, so it writes even while the gate refuses and never sees a switch. Pre-existing and outside this package — this PR does not close the contamination class, and it deserves its own issue.contextRetriever,inconsistencyDetector,mergeCandidateDetectorandtopicDetectorresolveembeddingClientthemselves. Pre-existing; the gate governs the knowledge graph's own writes.embed()— the fence prevents its result from landing, which is the part that matters, but the provider call still happens and is still billed.Review history
Ten implementation rounds across two phases, gated by eight adversarial review passes (a repo-contract reviewer and a cross-vendor auditor run independently each time). Twenty-two blocking defects found and fixed.
The two most instructive: the first cut of the gate did not actually gate — it sampled existing rows instead of reading the column width, so a fresh install with a 1536-dim model would have run permanently FTS-only while reporting healthy. And the write fence was missing entirely until a reviewer noticed that
EmbeddingBackfillHandle.stop()only clears timers, so a test that stubbedsyncBackfillhad been green for four rounds over a real contamination bug.Closes #440