docs: address codex review in esmole spec
This commit is contained in:
@@ -74,24 +74,51 @@ storePath/envVar/logPrefix as dependencies.
|
|||||||
- `createRegistry({ storePath, configPath, env, schema, logPrefix, envVar })` —
|
- `createRegistry({ storePath, configPath, env, schema, logPrefix, envVar })` —
|
||||||
schema injected; no direct import of any concrete schema.
|
schema injected; no direct import of any concrete schema.
|
||||||
- `createManager<TBackend extends { dispose(): Promise<void> }>(registry,
|
- `createManager<TBackend extends { dispose(): Promise<void> }>(registry,
|
||||||
{ createBackend, createTunnel })` — generic over the backend; does not know
|
{ createBackend, createTunnel, resolvePort })` — generic over the backend; does
|
||||||
`Driver`.
|
not know `Driver`. `resolvePort(config)` is injected because the manager itself
|
||||||
- `baseConnectionShape` — a zod raw shape **without** `type`. Each package spreads
|
calls `defaultPort(config.type)` today (`manager.ts:67`); ES needs 9200, SQL
|
||||||
it, adds its own `type` enum and engine-specific fields, then calls `.strict()`.
|
5432/3306.
|
||||||
|
- `baseConnectionShape` — a zod raw shape **without** `type`, **including** the
|
||||||
|
`ssh` field (`sshConfigSchema` moves to core; the tunnel is already SQL-free
|
||||||
|
except for the `SshConfig` type at `tunnel.ts:5`). Each package spreads it, adds
|
||||||
|
its own `type` enum and engine-specific fields, then calls `.strict()`.
|
||||||
- `openTunnel` / `Tunnel` — unchanged (pure TCP).
|
- `openTunnel` / `Tunnel` — unchanged (pure TCP).
|
||||||
- `withManaged`, `respond` — unchanged, already generic.
|
- `respond` — unchanged, already generic.
|
||||||
- `registerConnectionTools(server, { manager, registry, schema, ping })` — generic
|
- `withManaged<TBackend, TConfig>(manager, name, fn, { isStaleError, formatError })`
|
||||||
connection CRUD (list / add / remove / update / test_connection); `ping(backend)`
|
— generic. It is **not** unchanged: today it imports SQL `DriverDisposedError`,
|
||||||
is the per-backend hook backing `test_connection`.
|
`ManagedConnection`, and `formatDbError(config.type, …)` (`managed.ts:25,34`).
|
||||||
- format primitives: `clampLimit`, `truncateRows`, `truncateJsonBudget`.
|
Core exports a backend-neutral stale-error class; stale detection and error
|
||||||
|
formatting are injected by each package (or stale-retry moves into the manager
|
||||||
|
behind that neutral error).
|
||||||
|
- `registerConnectionTools(server, { manager, registry, fullSchema, patchSchema,
|
||||||
|
publicView, descriptions, ping, formatError })` — generic connection CRUD
|
||||||
|
(list / add / remove / update / test_connection). The current tools bake in SQL
|
||||||
|
patch fields, the SQL public view (`database`), SQL default-port rendering, SQL
|
||||||
|
descriptions, `serverVersion()`, and SQL error formatting
|
||||||
|
(`connections.ts:21,61,131`); all of these are package-owned and injected. Core
|
||||||
|
only orchestrates registry + manager calls. `ping(backend)` backs
|
||||||
|
`test_connection` (SQL `serverVersion()` vs ES `GET /`).
|
||||||
|
- format split: only truncation / token-budget helpers go to core (`clampLimit`,
|
||||||
|
`truncateRows`, `truncateJsonBudget`). SQL-shaped `normalizeCell` /
|
||||||
|
`formatDbError` (`format.ts:16,36`) stay in dbmole-mcp.
|
||||||
|
|
||||||
### §3. Manager generalization
|
### §3. Manager generalization
|
||||||
|
|
||||||
The manager needs only `dispose()` from a backend. All the hard logic — cache,
|
The manager needs only `dispose()` from a backend. All the hard logic — cache,
|
||||||
rotation, dispose-race handling, tunnel guards, retry-on-stale (`manager.ts:82-129`)
|
rotation, dispose-race handling, tunnel guards, retry-on-stale (`manager.ts:82-129`)
|
||||||
— moves verbatim into core. The only change: `defaultCreateDriver` becomes the
|
— moves verbatim into core. Changes: `defaultCreateDriver` becomes the injected
|
||||||
injected `createBackend(target)`. `DriverTarget` → `BackendTarget { config, host,
|
`createBackend(target)`; the internal `defaultPort(config.type)` call
|
||||||
port }` (generic config type parameter). The `tunnel?.isClosed()` recheck stays.
|
(`manager.ts:67`) becomes the injected `resolvePort(config)`. The
|
||||||
|
`tunnel?.isClosed()` recheck stays.
|
||||||
|
|
||||||
|
`DriverTarget` → `BackendTarget { config, connectHost, connectPort, serverName }`
|
||||||
|
(generic config type parameter). `connectHost`/`connectPort` are where the client
|
||||||
|
actually dials — the tunnel's `127.0.0.1:localPort` when tunneled, else the real
|
||||||
|
host/port. `serverName` is the original `config.host`, carried through for TLS SNI
|
||||||
|
/ certificate hostname verification. Without this split, HTTPS Elasticsearch over
|
||||||
|
an SSH tunnel fails `verifyTls`, because the cert covers the real host, not
|
||||||
|
`127.0.0.1` (`tunnel.ts:170`). SQL drivers ignore `serverName`; the ES client sets
|
||||||
|
it as the TLS servername.
|
||||||
|
|
||||||
The SQL `Driver` interface stays in `dbmole-mcp`. ES implements its own `Backend`.
|
The SQL `Driver` interface stays in `dbmole-mcp`. ES implements its own `Backend`.
|
||||||
Both satisfy `{ dispose(): Promise<void> }`, so both ride the same manager — ES
|
Both satisfy `{ dispose(): Promise<void> }`, so both ride the same manager — ES
|
||||||
@@ -116,17 +143,24 @@ and the store for free (an upgrade over the single-connection reference).
|
|||||||
|
|
||||||
### §5. esmole backend + tools
|
### §5. esmole backend + tools
|
||||||
|
|
||||||
**Backend** = an HTTP client (undici, keep-alive + `dispose()`) bound to
|
**Backend** = an HTTP client (undici, keep-alive + `dispose()`) dialing
|
||||||
`scheme://tunnelHost:tunnelPort`, with auth (basic or apiKey) and `verifyTls`.
|
`scheme://connectHost:connectPort` (the tunnel endpoint when tunneled), with auth
|
||||||
|
(basic or apiKey) and `verifyTls`. When `verifyTls` is on and the connection is
|
||||||
|
tunneled, the client sets the TLS servername to `BackendTarget.serverName` (the
|
||||||
|
real ES host) so certificate hostname verification passes (see §3).
|
||||||
`request(method, path, { body, params })` → `{status, body}`, never throwing on
|
`request(method, path, { body, params })` → `{status, body}`, never throwing on
|
||||||
4xx/5xx; body parsed as JSON when possible, else text. A `string` body is sent
|
4xx/5xx; body parsed as JSON when possible, else text. A `string` body is sent
|
||||||
as-is (for NDJSON `_bulk`); dict/list is JSON-serialized.
|
as-is (for NDJSON `_bulk`); dict/list is JSON-serialized.
|
||||||
|
|
||||||
**readonly guard** (`es/guard.ts`, role analogous to `sqlGuard`): when the
|
**readonly guard** (`es/guard.ts`, role analogous to `sqlGuard`): a method+path
|
||||||
connection is `readonly`, allow only GET/HEAD plus POST to a read-suffix allowlist
|
boundary. When the connection is `readonly`, allow GET/HEAD plus POST to a
|
||||||
(`_search`, `_msearch`, `_count`, `_field_caps`, `_cat`, `_mapping`, `scroll`).
|
read-suffix allowlist (`_search`, `_msearch`, `_count`, `_field_caps`, `_cat`,
|
||||||
Block all PUT/DELETE and any other POST. Allowlist (not blocklist) so unknown
|
`_mapping`, `_search/scroll`, `_pit`) plus DELETE limited to `_pit` and
|
||||||
endpoints fail safe.
|
`_search/scroll` (point-in-time / scroll cleanup — read-session teardown, not data
|
||||||
|
mutation). Block all other PUT/DELETE and any other POST. `_sql` is blocked by
|
||||||
|
absence from the allowlist (it can write). Allowlist (not blocklist) so unknown
|
||||||
|
endpoints fail safe. Script content inside a `_search` body is content-level, not
|
||||||
|
method-level, and is out of scope for this guard.
|
||||||
|
|
||||||
**Tools (5 ES-specific + connection CRUD from core):**
|
**Tools (5 ES-specific + connection CRUD from core):**
|
||||||
|
|
||||||
@@ -159,6 +193,9 @@ mirroring dbmole's row truncation.
|
|||||||
esmole integration runs against ES 7.x **and** 8.x containers. Coverage ≥90%
|
esmole integration runs against ES 7.x **and** 8.x containers. Coverage ≥90%
|
||||||
lines/functions per package — thresholds never lowered.
|
lines/functions per package — thresholds never lowered.
|
||||||
- Manager concurrency tests move into `core` alongside the manager.
|
- Manager concurrency tests move into `core` alongside the manager.
|
||||||
|
- Remap / alias test import paths **before** moving files, not after. Integration
|
||||||
|
tests reference the old `src/...` paths; if files move first, the ≥90% gate
|
||||||
|
breaks mid-migration.
|
||||||
|
|
||||||
## Defaults
|
## Defaults
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user