History

fix patch release data upgrades (#405)

Rhys Sullivan committed 6 months ago

Commit 1492de31 parent 28ea43c6 files · +166 −16Browse files

Unified · Split

6 files
  1. .changeset/data-upgrade-paths.md +5 −0
  2. .skills/cli-release/SKILL.md +3 −1
  3. apps/cli/release-notes/next.md +2 −0
  4. apps/local/src/server/migrate-connections.test.ts +115 −0
  5. apps/local/src/server/migrate-connections.ts +29 −11
  6. packages/plugins/openapi/src/sdk/plugin.ts +12 −4
.changeset/data-upgrade-paths.md+5−0
@@ -0,0 +1,5 @@
1---
2"executor": patch
3---
4
5Fix local data upgrade paths for legacy OAuth connection backfills and OpenAPI inline credential rewrites.
.skills/cli-release/SKILL.md+3−1
@@ -26,7 +26,9 @@ Does **not** ship in the CLI: · 25 unmodified lines
26 26
27- Prior convention in this repo uses **`patch`** bumps for feature-heavy releases (see `.changeset/executor-1.4.6-beta.md` for precedent). Don't push back on patch unless there are genuine SemVer-breaking API changes to a library consumer surface.27- Prior convention in this repo uses **`patch`** bumps for feature-heavy releases (see `.changeset/executor-1.4.6-beta.md` for precedent). Don't push back on patch unless there are genuine SemVer-breaking API changes to a library consumer surface.
28- Breaking CLI UX changes (removed flags, changed argv shape) have historically still been `patch` bumps. Follow the owner's call — ask, don't assume `minor`.28- Breaking CLI UX changes (removed flags, changed argv shape) have historically still been `patch` bumps. Follow the owner's call — ask, don't assume `minor`.
29- Only `apps/cli/package.json` version should move during Version Packages PRs. `@executor-js/*` library packages have their own publish path.29- Normal release/patch PRs must add a `.changeset/*.md` file with frontmatter like `"executor": patch`. Do **not** directly bump `apps/cli/package.json` or `bun.lock` in a feature/fix PR.
30- Only the Changesets-generated `Version Packages` PR should move `apps/cli/package.json`. If a normal PR directly changes that version, merging it to `main` can make `.github/workflows/release.yml` tag the commit and dispatch `publish-executor-package.yml`, causing an immediate CLI publish.
31- `@executor-js/*` library packages have their own publish path.
30 32
31## Release notes: curated, not auto-generated33## Release notes: curated, not auto-generated
32 34
apps/cli/release-notes/next.md+2−0
@@ -50,6 +50,8 @@ Tool dispatch, plugins, storage, schema, and transport are now fully instrumente · 49 unmodified lines
50- Per-scope blob and secret lookups now use a single `IN` query instead of N per-scope round-trips.50- Per-scope blob and secret lookups now use a single `IN` query instead of N per-scope round-trips.
51 51
52## Fixes52## Fixes
53- Upgrade: preserve legacy OAuth connection backfills after the `connection.kind` column is removed.
54- OpenAPI: refreshing or editing sources with legacy inline secret/OAuth config now materializes the new source binding rows instead of dropping credentials.
53- Keychain: skip provider registration when the OS backend is unreachable (no more startup failure when running headless on Linux without a keyring).55- Keychain: skip provider registration when the OS backend is unreachable (no more startup failure when running headless on Linux without a keyring).
54- Local server: return 404 for missing static assets instead of serving HTML.56- Local server: return 404 for missing static assets instead of serving HTML.
55- Tests: Windows compatibility across the suite.57- Tests: Windows compatibility across the suite.
apps/local/src/server/migrate-connections.test.ts+115−0
@@ -0,0 +1,115 @@
1import { afterEach, beforeEach, describe, expect, it } from "@effect/vitest";
2import { Database } from "bun:sqlite";
3import { migrate } from "drizzle-orm/bun-sqlite/migrator";
4import { drizzle } from "drizzle-orm/bun-sqlite";
5import { mkdtempSync, rmSync } from "node:fs";
6import { tmpdir } from "node:os";
7import { join } from "node:path";
8
9import { migrateLegacyConnections } from "./migrate-connections";
10
11let workDir: string;
12
13beforeEach(() => {
14 workDir = mkdtempSync(join(tmpdir(), "executor-migrate-connections-"));
15});
16
17afterEach(() => {
18 rmSync(workDir, { recursive: true, force: true });
19});
20
21const columnNames = (db: Database, table: string): ReadonlyArray<string> =>
22 (
23 db.prepare(`PRAGMA table_info('${table}')`).all() as ReadonlyArray<{
24 readonly name: string;
25 }>
26 ).map((column) => column.name);
27
28describe("migrateLegacyConnections", () => {
29 it("backfills legacy MCP OAuth rows after connection.kind has been dropped", async () => {
30 const db = new Database(join(workDir, "data.db"));
31 migrate(drizzle(db), {
32 migrationsFolder: join(import.meta.dirname, "../../drizzle"),
33 });
34
35 expect(columnNames(db, "connection")).not.toContain("kind");
36
37 const now = Date.now();
38 db.prepare(
39 "INSERT INTO secret (scope_id, id, name, provider, created_at) VALUES (?, ?, ?, ?, ?)",
40 ).run("scope-1", "access-token", "Access token", "keychain", now);
41 db.prepare(
42 "INSERT INTO secret (scope_id, id, name, provider, created_at) VALUES (?, ?, ?, ?, ?)",
43 ).run("scope-1", "refresh-token", "Refresh token", "keychain", now);
44 db.prepare(
45 "INSERT INTO mcp_source (scope_id, id, name, config, created_at) VALUES (?, ?, ?, ?, ?)",
46 ).run(
47 "scope-1",
48 "remote-mcp",
49 "Remote MCP",
50 JSON.stringify({
51 transport: "remote",
52 endpoint: "https://example.com/mcp",
53 auth: {
54 kind: "oauth2",
55 accessTokenSecretId: "access-token",
56 refreshTokenSecretId: "refresh-token",
57 tokenType: "Bearer",
58 expiresAt: null,
59 scope: "read",
60 clientInformation: null,
61 authorizationServerUrl: null,
62 resourceMetadataUrl: null,
63 },
64 }),
65 now,
66 );
67
68 await migrateLegacyConnections(db);
69
70 const connection = db
71 .prepare(
72 "SELECT id, provider, access_token_secret_id, refresh_token_secret_id FROM connection WHERE scope_id = ?",
73 )
74 .get("scope-1") as
75 | {
76 readonly id: string;
77 readonly provider: string;
78 readonly access_token_secret_id: string;
79 readonly refresh_token_secret_id: string | null;
80 }
81 | undefined;
82 expect(connection).toEqual({
83 id: "mcp-oauth2-remote-mcp",
84 provider: "mcp:oauth2",
85 access_token_secret_id: "access-token",
86 refresh_token_secret_id: "refresh-token",
87 });
88
89 const source = db
90 .prepare("SELECT config FROM mcp_source WHERE scope_id = ? AND id = ?")
91 .get("scope-1", "remote-mcp") as { readonly config: string };
92 expect(JSON.parse(source.config).auth).toEqual({
93 kind: "oauth2",
94 connectionId: "mcp-oauth2-remote-mcp",
95 });
96
97 const ownedSecrets = db
98 .prepare(
99 "SELECT id, owned_by_connection_id FROM secret WHERE scope_id = ? ORDER BY id",
100 )
101 .all("scope-1");
102 expect(ownedSecrets).toEqual([
103 {
104 id: "access-token",
105 owned_by_connection_id: "mcp-oauth2-remote-mcp",
106 },
107 {
108 id: "refresh-token",
109 owned_by_connection_id: "mcp-oauth2-remote-mcp",
110 },
111 ]);
112
113 db.close();
114 });
115});
apps/local/src/server/migrate-connections.ts+29−11
@@ -53,6 +53,13 @@ const tableExists = (sqlite: Database, name: string): boolean => { · 52 unmodified lines
53 return row !== null && row !== undefined;53 return row !== null && row !== undefined;
54};54};
55 55
56const columnExists = (sqlite: Database, table: string, column: string): boolean => {
57 const columns = sqlite
58 .prepare(`PRAGMA table_info('${table.replaceAll("'", "''")}')`)
59 .all() as ReadonlyArray<{ readonly name: string }>;
60 return columns.some((c) => c.name === column);
61};
62
56type SecretRow = { id: string; owned_by_connection_id: string | null };63type SecretRow = { id: string; owned_by_connection_id: string | null };
57 64
58/** Shared: re-parent the pointed-to secret ids to the new connection,65/** Shared: re-parent the pointed-to secret ids to the new connection,
@@ -133,20 +140,30 @@ const insertConnectionRow = ( · 74 unmodified lines
133 providerState: unknown;140 providerState: unknown;
134 },141 },
135): void => {142): void => {
136 const stmt = sqlite.prepare(143 const hasKind = columnExists(sqlite, "connection", "kind");
137 `INSERT INTO connection (144 const stmt = hasKind
138 id, scope_id, provider, kind, identity_label,145 ? sqlite.prepare(
139 access_token_secret_id, refresh_token_secret_id,146 `INSERT INTO connection (
140 expires_at, scope, provider_state,147 id, scope_id, provider, kind, identity_label,
141 created_at, updated_at148 access_token_secret_id, refresh_token_secret_id,
142 ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,149 expires_at, scope, provider_state,
143 );150 created_at, updated_at
151 ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
152 )
153 : sqlite.prepare(
154 `INSERT INTO connection (
155 id, scope_id, provider, identity_label,
156 access_token_secret_id, refresh_token_secret_id,
157 expires_at, scope, provider_state,
158 created_at, updated_at
159 ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
160 );
144 const now = Date.now();161 const now = Date.now();
145 stmt.run(162 const values = [
146 params.id,163 params.id,
147 params.scopeId,164 params.scopeId,
148 params.provider,165 params.provider,
149 "user",166 ...(hasKind ? ["user"] : []),
150 params.identityLabel,167 params.identityLabel,
151 params.accessTokenSecretId,168 params.accessTokenSecretId,
152 params.refreshTokenSecretId,169 params.refreshTokenSecretId,
@@ -155,7 +172,8 @@ const insertConnectionRow = ( · 2 unmodified lines
155 JSON.stringify(params.providerState),172 JSON.stringify(params.providerState),
156 now,173 now,
157 now,174 now,
158 );175 ];
176 stmt.run(...values);
159};177};
160 178
161// ---------------------------------------------------------------------------179// ---------------------------------------------------------------------------
packages/plugins/openapi/src/sdk/plugin.ts+12−4
@@ -795,8 +795,8 @@ export const openApiPlugin = definePlugin( · 794 unmodified lines
795 name: existing.name,795 name: existing.name,
796 baseUrl: resolvedConfig.baseUrl,796 baseUrl: resolvedConfig.baseUrl,
797 namespace: existing.namespace,797 namespace: existing.namespace,
798 headers: existing.config.headers,798 headers: existing.legacy?.headers ?? existing.config.headers,
799 oauth2: existing.config.oauth2,799 oauth2: existing.legacy?.oauth2 ?? existing.config.oauth2,
800 });800 });
801 });801 });
802 802
@@ -872,9 +872,17 @@ export const openApiPlugin = definePlugin( · 69 unmodified lines
872 const existing = yield* ctx.storage.getSource(namespace, scope);872 const existing = yield* ctx.storage.getSource(namespace, scope);
873 if (!existing) return;873 if (!existing) return;
874 const canonicalHeaders =874 const canonicalHeaders =
875 input.headers !== undefined ? canonicalizeHeaders(input.headers) : null;875 input.headers !== undefined
876 ? canonicalizeHeaders(input.headers)
877 : existing.legacy?.headers
878 ? canonicalizeHeaders(existing.legacy.headers)
879 : null;
876 const canonicalOAuth2 =880 const canonicalOAuth2 =
877 input.oauth2 !== undefined ? canonicalizeOAuth2(input.oauth2) : null;881 input.oauth2 !== undefined
882 ? canonicalizeOAuth2(input.oauth2)
883 : existing.legacy?.oauth2
884 ? canonicalizeOAuth2(existing.legacy.oauth2)
885 : null;
878 yield* ctx.storage.updateSourceMeta(namespace, scope, {886 yield* ctx.storage.updateSourceMeta(namespace, scope, {
879 name: input.name?.trim() || undefined,887 name: input.name?.trim() || undefined,
880 baseUrl: input.baseUrl,888 baseUrl: input.baseUrl,