HarperFast / HarperFast/harper
models: backend registry replacement orphans the old backend — no dispose lifecycle on ModelBackend
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Problem
The backend registry (`resources/models/backendRegistry.ts`) replaces registrations with a plain `Map.set()` — re-registering a logical name (or `clearRegistry()`) drops the old `ModelBackend` instance without any notification. For the built-in HTTP backends that's just garbage collection. For **in-process backends** — the case #1325/#1471 explicitly support — the instance can own real native resources: a config-selectable GGUF embedding backend holds llama.cpp model weights and a decode context (heskew/harper-fabric-embeddings#3). Silent replacement leaks them for the life of the process, and process shutdown never notifies backends either.
**Severity framing (honest):** today `bootstrapModels` runs from a single boot-time call site (`componentLoader.ts:373`, root config only), so replacement effectively doesn't happen in production yet — the leak is latent. But the contract gap is real for (a) any future config hot-reload of the `models:` block, (b) apps that call `registerBackend()` more than once per process (component reload paths), and (c) graceful shutdown of backends with connections/handles.
## Sketch
Optional lifecycle method on the contract:
```ts
interface ModelBackend {
// ...
/** Release resources. Called by the registry when this instance is replaced or cleared. */
dispose?(): Promise | void;
}
```
Registry behavior:
- `setEmbedding`/`setGenerative` (and therefore `registerBackend`): when the slot already holds a **different** instance, call the old instance's `dispose()` after the swap — fire-and-forget with error logging, so a failing dispose can't break registration.
- `clearRegistry()`: dispose all, then clear.
- Optionally: a shutdown hook wired to the same place component scopes get closed.
**The subtlety worth deciding up front:** one backend instance can legitimately be registered under multiple logical names (aliases pointing at one engine). Name-slot replacement must not dispose an instance still referenced by another name — the registry needs an identity check across both kind-maps before disposing (or we document one-name-per-instance as a contract). This is the main reason to design this in core rather than have each backend hand-roll it.
`defineBackend` passes `dispose` through from the spec.
Related: #1325 (registerBackend/defineBackend), #1471 (config-selectable module backends), #1235 (models subsystem audit).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Contributor guide
Research direction
Start in resources/models/backendRegistry.ts, then inspect defineBackend and the bootstrapModels call site at componentLoader.ts:373. Trace setEmbedding, setGenerative, registerBackend, and clearRegistry before deciding how replacement and clearing should invoke disposal without breaking registration. Done means the lifecycle contract and registry behavior cover replacement, clearing, alias identity, and disposal errors; shutdown integration is an explicitly resolved part of the scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100