PR #172 fixed the jsonb double-encoding for `tool_allowlist` but the same class of bug, and the same re-derived workaround, remained elsewhere. 1. model_config (agent roles): jsonbObject still used the buggy `::jsonb` bind, so `ai_agent_roles.model_config` round-tripped as a jsonb STRING SCALAR. The read-path `typeof === 'object'` check then failed and the model override was SILENTLY dropped (role fell back to the default model). Fixed to `::text::jsonb` and added `parseModelConfig` + `normalizeRow` so every read self-heals already-corrupted rows (no migration). 2. Centralized the write workaround as `jsonbBind()` in database/utils.ts — one implementation with one explanation of the quirk — replacing the per-repo `jsonbArray` (mcp) and `jsonbObject` (roles). 3. Integration coverage (the fix is a DB round-trip a unit test cannot see; the read-side parser MASKS a write regression): new ai-mcp-server-repo.int-spec asserts `jsonb_typeof(tool_allowlist)='array'` after insert + heals a seeded string-scalar row; ai-agent-roles-repo int-spec gains the same for `model_config` (`'object'` + heal). 4. Updated the stale `ai-mcp-servers.types.ts` comment (the driver returns a JSON string for legacy rows; the repo normalizes every read). 5. Fail-open logging: a corrupt tool_allowlist degrades to "no restriction" (agent gets ALL tools) — normalizeRow now warns (server id only, never contents) so the silent widening leaves a trace. 6. Simplified parseToolAllowlist (normalize the string once, then a single array-of-strings check) — identical behaviour, all 12 cases still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import { sql, RawBuilder } from 'kysely';
|
||||
import { KyselyDB, KyselyTransaction } from './types/kysely.types';
|
||||
|
||||
/*
|
||||
@@ -31,3 +32,35 @@ export function dbOrTx(
|
||||
return db; // Use normal database instance
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Bind a JS array/object as a `jsonb` column value, working around a postgres
|
||||
* driver double-encoding quirk. THE single implementation — repos that persist
|
||||
* jsonb (`tool_allowlist`, `model_config`, ...) call this instead of re-deriving
|
||||
* the cast.
|
||||
*
|
||||
* THE QUIRK: with the `kysely-postgres-js` / postgres.js driver, casting a bound
|
||||
* parameter straight to `::jsonb` makes the driver infer the param type as jsonb
|
||||
* and JSON-stringify the (already-JSON) text a SECOND time, so the column ends
|
||||
* up holding a jsonb STRING SCALAR (`"[\"a\"]"` / `"{\"k\":1}"`) instead of a
|
||||
* real jsonb array/object. Read paths then see a string, not the structure, and
|
||||
* silently fall back (an allowlist becomes "unrestricted", a model override is
|
||||
* ignored). Forcing the param through `::text` first binds it as text (sent
|
||||
* verbatim); `::jsonb` then parses it into a real array/object. Read-side
|
||||
* parsers repair rows written the old buggy way without a migration.
|
||||
*
|
||||
* Returns `null` for null/undefined and for "empty" values (an empty array, or
|
||||
* an object with no own enumerable keys) — callers treat empty as "clear/unset",
|
||||
* so an empty allowlist/config never round-trips as `[]`/`{}`.
|
||||
*/
|
||||
export function jsonbBind<T>(
|
||||
value: T | null | undefined,
|
||||
): RawBuilder<T> | null {
|
||||
if (value === null || value === undefined) return null;
|
||||
if (Array.isArray(value)) {
|
||||
if (value.length === 0) return null;
|
||||
} else if (typeof value === 'object') {
|
||||
if (Object.keys(value as object).length === 0) return null;
|
||||
}
|
||||
return sql<T>`${JSON.stringify(value)}::text::jsonb`;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user