mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
fix(db): migrate connector hooks on SQL_ASCII databases (#16958)
## Thinking Path > - Paperclip manages work for AI agents. > - Server startup applies database migrations before accepting work. > - Some older embedded databases use SQL_ASCII encoding. > - Migration 0355 uses a Unicode escape literal that PostgreSQL rejects on those databases. > - A source rebuild can pass but the updated server cannot start. > - This change makes that migration portable and preserves its shipped receipt. > - Existing databases retain their newer connector triggers during upgrades. ## Linked Issues or Issue Description **What happened?** A source deployment failed at startup with `conversion between UTF8 and SQL_ASCII is not supported`. Migration 0355 could not compile its connector policy function on an older embedded database. The previous app was restored while this repair was prepared. **Expected behavior** Supported legacy databases complete startup migrations. Connector selector trimming matches JavaScript `trim()`. A database that already applied 0355 must not replay it over the later 0356 connector trigger definition. **Steps to reproduce** Create a PostgreSQL database from template0 with SQL_ASCII encoding, then apply the current migration chain. The new regression fails with the observed conversion error before this change. **Paperclip version or commit** Reproduced on `92b238137ba716cdf1746dd49969e2f5ab7af8fe`. The branch includes current master `f57ed25385`. **Database mode** Embedded PostgreSQL 18.1; legacy SQL_ASCII database. The UTF-8 path is also covered. Related background: #645 fixed encoding for new databases. Existing SQL_ASCII installations still need portable migrations. #16956 fixes a separate workspace lock timeout that motivated this deployment. ## What Changed - Build the whitespace pattern from UTF-8 bytes instead of a Unicode escape literal. - Match whole whitespace sequences so SQL_ASCII does not trim partial multibyte characters. - Recognize the original 0355 checksum as a valid receipt. This prevents replay of its older trigger body over 0356. - Add real PostgreSQL coverage for both encodings, selector behavior, and preservation of the later trigger definition and receipt count. - Document the legacy encoding and migration-history contract. ## Verification - Red: SQL_ASCII migration failed with the same conversion error; UTF-8 passed. - Second red: fixing only the SQL caused an existing receipt to replay and overwrite the later connector function. - Green: 29 focused tests passed across the new regression, migration replay tests, and database client tests. - Rebased regression: all 29 focused tests passed. - `pnpm -r typecheck`, `pnpm build`, and `pnpm --filter @paperclipai/db check:migrations` passed. - Linux ARM64: clean build and 37 focused tests passed, including both encoding and workspace-lock regressions. - Live acceptance: the same legacy SQL_ASCII database completed all seven remaining migrations. The server reports the tested commit and healthy status. The restart report has no lost runs. - All 55 current-head checks passed; two unrelated Storybook checks were skipped. Full test, typecheck, build, runner, release, and E2E CI gates passed. The complete suite ran in CI; local verification used focused tests. - Greptile reviewed this exact commit at 5/5 with no findings. - Preserved failures: the initial unmodified-master startup failed at migration 0355 and was rolled back. A later build-cache path error in the deployment commands was corrected before the successful clean build; it required no source change. ## Risks The migration content changes, so receipt compatibility matters. The original checksum remains recognized to avoid replaying historical DDL. The tests compare complete later function definitions and receipt counts. The whitespace pattern contains encoded bytes; tests cover every ECMAScript whitespace character and preserve non-whitespace Unicode input. Database data and schema shape are unchanged by this repair. ## Model Used OpenAI GPT-6 through Codex, with repository tools, code execution, and browser verification. The exact served model variant and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
7c247243e5
commit
96824a71d1
4 files changed
+92
-3
No files matched your search
@@ -342,6 +342,14 @@ Your Drizzle schema (`packages/db/src/schema/`) stays the same regardless of mod
|
||||
|
||||
## Migration authoring checklist
|
||||
|
||||
Older embedded databases can use `SQL_ASCII`. Test encoding-sensitive SQL on
|
||||
both `SQL_ASCII` and UTF-8 databases. Migration `0355` constructs its whitespace
|
||||
pattern from UTF-8 bytes so legacy databases can compile it, and matches whole
|
||||
characters instead of trimming individual bytes. Its shipped hash remains a
|
||||
valid migration receipt; otherwise an upgrade could replay its older connector
|
||||
triggers over the replacements from `0356`. The encoding migration regression
|
||||
checks both selector behavior and preservation of that later trigger definition.
|
||||
|
||||
The 0126 issue comment attribution backfill showed the failure mode this checklist is meant to prevent: each batch looked for the next rows with an unindexed predicate, so PostgreSQL repeatedly scanned the same table and the migration became O(n²) as the table grew.
|
||||
|
||||
When authoring migrations or one-time backfills:
|
||||
|
||||
@@ -580,6 +580,9 @@ async function mapHashesToMigrationFiles(migrationFiles: string[]): Promise<Map<
|
||||
const content = await readMigrationFileContent(migrationFile);
|
||||
const hash = createHash("sha256").update(content).digest("hex");
|
||||
mapped.set(hash, migrationFile);
|
||||
if (migrationFile === "0355_agent_tool_definition_hooks.sql") {
|
||||
mapped.set("7ac76adac0220da522c78e51da20490aeb0f90512b4370a05af9f8dd0f75eda6", migrationFile);
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { readFile } from "node:fs/promises";
|
||||
import postgres from "postgres";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { applyPendingMigrations, inspectMigrations } from "./client.js";
|
||||
import {
|
||||
EMBEDDED_POSTGRES_TEST_TIMEOUT_MS,
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
type EmbeddedPostgresTestDatabase,
|
||||
} from "./test-embedded-postgres.js";
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
const describePostgres = support.supported ? describe : describe.skip;
|
||||
const migration = "0355_agent_tool_definition_hooks.sql";
|
||||
const company = "00000000-0000-4000-8000-000000000001";
|
||||
const agent = "00000000-0000-4000-8000-000000000002";
|
||||
const whitespace = "\t\n\v\f\r \u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff";
|
||||
|
||||
describePostgres("connector policy migration database encodings", () => {
|
||||
let database: EmbeddedPostgresTestDatabase | undefined;
|
||||
let utf8: ReturnType<typeof postgres> | undefined;
|
||||
let ascii: ReturnType<typeof postgres> | undefined;
|
||||
let asciiUrl: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
database = await startEmbeddedPostgresTestDatabase("paperclip-connector-encoding-");
|
||||
utf8 = postgres(database.connectionString, { max: 1, onnotice: () => {} });
|
||||
await utf8.unsafe("CREATE DATABASE legacy_ascii TEMPLATE template0 ENCODING 'SQL_ASCII' LC_COLLATE 'C' LC_CTYPE 'C'");
|
||||
const url = new URL(database.connectionString);
|
||||
url.pathname = "/legacy_ascii";
|
||||
asciiUrl = url.toString();
|
||||
ascii = postgres(asciiUrl, { max: 1, onnotice: () => {} });
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
|
||||
afterAll(async () => {
|
||||
try {
|
||||
await Promise.all([utf8?.end(), ascii?.end()]);
|
||||
} finally {
|
||||
await database?.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["UTF8", "SQL_ASCII"])("migrates %s and matches JavaScript selector trimming", async (encoding) => {
|
||||
const sql = encoding === "UTF8" ? utf8! : ascii!;
|
||||
if (encoding === "SQL_ASCII") await applyPendingMigrations(asciiUrl);
|
||||
expect((await sql`SHOW server_encoding`)[0]?.server_encoding).toBe(encoding);
|
||||
for (const input of [...whitespace, whitespace, `${whitespace}${agent}${whitespace}`, "\u200b", "é", "中"]) {
|
||||
const actual = await sql`SELECT * FROM paperclip_connector_policy_targets(${company}::uuid, ${sql.json({ agentIds: input })})`;
|
||||
expect(Array.from(actual), JSON.stringify(input)).toEqual(input.trim()
|
||||
? [{ target_type: "agent", target_id: input.trim() }]
|
||||
: [{ target_type: "company", target_id: company }]);
|
||||
}
|
||||
const arrays = await sql`SELECT * FROM paperclip_connector_policy_targets(${company}::uuid,
|
||||
${sql.json({ agentIds: [whitespace, agent, ` ${agent} `, null, 12] })})`;
|
||||
expect(Array.from(arrays)).toEqual([
|
||||
{ target_type: "agent", target_id: agent },
|
||||
{ target_type: "agent", target_id: ` ${agent} ` },
|
||||
]);
|
||||
const intersection = await sql`SELECT * FROM paperclip_connector_policy_targets(${company}::uuid,
|
||||
${sql.json({ agentId: company, agentIds: [agent] })})`;
|
||||
expect(Array.from(intersection)).toEqual([]);
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
|
||||
it("recognizes the shipped receipt without replaying older connector triggers", async () => {
|
||||
const sql = utf8!;
|
||||
const content = await readFile(new URL(`./migrations/${migration}`, import.meta.url), "utf8");
|
||||
const hash = createHash("sha256").update(content).digest("hex");
|
||||
const originalHash = "7ac76adac0220da522c78e51da20490aeb0f90512b4370a05af9f8dd0f75eda6";
|
||||
const before = await sql`SELECT pg_get_functiondef('paperclip_connector_runtime_changed()'::regprocedure) AS definition`;
|
||||
const count = await sql`SELECT count(*)::int AS count FROM drizzle.__drizzle_migrations`;
|
||||
await sql`UPDATE drizzle.__drizzle_migrations SET hash = ${originalHash} WHERE hash = ${hash}`;
|
||||
await applyPendingMigrations(database!.connectionString);
|
||||
expect((await inspectMigrations(database!.connectionString)).status).toBe("upToDate");
|
||||
expect(await sql`SELECT pg_get_functiondef('paperclip_connector_runtime_changed()'::regprocedure) AS definition`).toEqual(before);
|
||||
expect(await sql`SELECT count(*)::int AS count FROM drizzle.__drizzle_migrations`).toEqual(count);
|
||||
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
||||
});
|
||||
@@ -3,13 +3,13 @@ RETURNS TABLE(target_type text, target_id text)
|
||||
LANGUAGE plpgsql IMMUTABLE SET search_path = public, pg_temp AS $$
|
||||
DECLARE single_agent text := CASE WHEN jsonb_typeof(selectors->'agentId') = 'string' THEN nullif(selectors->>'agentId', '') END;
|
||||
many_agents text[];
|
||||
trim_chars text := U&'\0009\000A\000B\000C\000D\0020\00A0\1680\2000\2001\2002\2003\2004\2005\2006\2007\2008\2009\200A\2028\2029\202F\205F\3000\FEFF';
|
||||
trim_pattern text := '[\x09-\x0d ]|' || convert_from(decode('c2a07ce19a807ce280807ce280817ce280827ce280837ce280847ce280857ce280867ce280877ce280887ce280897ce2808a7ce280a87ce280a97ce280af7ce2819f7ce380807cefbbbf', 'hex'), 'UTF8');
|
||||
BEGIN
|
||||
SELECT array_agg(value#>>'{}') INTO many_agents
|
||||
FROM jsonb_array_elements(CASE jsonb_typeof(selectors->'agentIds')
|
||||
WHEN 'array' THEN selectors->'agentIds'
|
||||
WHEN 'string' THEN jsonb_build_array(btrim(selectors->>'agentIds', trim_chars)) ELSE '[]'::jsonb END) value
|
||||
WHERE jsonb_typeof(value) = 'string' AND btrim(value#>>'{}', trim_chars) <> '';
|
||||
WHEN 'string' THEN jsonb_build_array(regexp_replace(selectors->>'agentIds', '^(' || trim_pattern || ')+|(' || trim_pattern || ')+$', '', 'g')) ELSE '[]'::jsonb END) value
|
||||
WHERE jsonb_typeof(value) = 'string' AND regexp_replace(value#>>'{}', '^(' || trim_pattern || ')+|(' || trim_pattern || ')+$', '', 'g') <> '';
|
||||
IF single_agent IS NOT NULL THEN
|
||||
IF coalesce(cardinality(many_agents), 0) = 0 OR single_agent = ANY(many_agents) THEN
|
||||
RETURN QUERY SELECT 'agent'::text, single_agent;
|
||||
|
||||
Reference in new issue
Block a user