Skip to content

Commit f7c651f

Browse files
RhysSullivanRhysSullivan
committed
Export 269c9c0
1 parent 4b889e6 commit f7c651f

6 files changed

Lines changed: 251 additions & 1 deletion

File tree

‎.changeset/app-removal-event-subscriptions.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@executor-js/sdk": patch
3+
---
4+
5+
Removing an app, or an owner with its apps, deletes the app's event subscriptions, their
6+
pending deliveries and its stored events. Subscribing to an app re-created with the same name
7+
no longer fails because the old subscription pinned that name to the removed app.

‎apps/hosted/server/src/implementation/mcp.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { appTargets, type McpBackend } from "@executor-js/mcp";
1515
import {
1616
AppNotFound,
1717
ElicitationFailed,
18+
EventNotFound,
1819
type AppId,
1920
type ToolInvocationOptions,
2021
} from "@executor-js/sdk/core";
@@ -131,7 +132,10 @@ export const hostedMcpBackend = Effect.gen(function* () {
131132
subscribeEvent: ({ key, ...input }) =>
132133
Effect.gen(function* () {
133134
const { principal, subject } = yield* subscriber;
134-
yield* eventApp(input.target.app);
135+
// A saved subscription can name an app the caller no longer sees; to them it has no event.
136+
yield* eventApp(input.target.app).pipe(
137+
Effect.catchTag("AppNotFound", () => Effect.fail(new EventNotFound({ kind: "event" }))),
138+
);
135139
return yield* (yield* sdk).events.subscribe({
136140
...input,
137141
key: { ...key, principal },

‎e2e/test-plan.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5198,6 +5198,28 @@ export const scenarios = plan({
51985198
local: na("This scenario uses hosted personal access token and webhook routes."),
51995199
},
52005200
},
5201+
mcpEventsAppRemoval: {
5202+
fixtures: "actors",
5203+
file: "mcp-events-app-removal.spec.ts",
5204+
title:
5205+
"removing an app ends its event subscriptions, and a subscription to a new app with the same name binds to the new app",
5206+
targets: {
5207+
"self-host": scheduled,
5208+
cloud: scheduled,
5209+
local: na("This scenario uses hosted personal access token and webhook routes."),
5210+
},
5211+
},
5212+
mcpEventsAppHidden: {
5213+
fixtures: "actors",
5214+
file: "mcp-events-app-removal.spec.ts",
5215+
title:
5216+
"a refresh by a caller who can no longer see the event's app is refused as an unknown event",
5217+
targets: {
5218+
"self-host": scheduled,
5219+
cloud: scheduled,
5220+
local: na("This scenario uses hosted personal access token and webhook routes."),
5221+
},
5222+
},
52015223
mcpEventGrants: {
52025224
fixtures: "actors",
52035225
file: "mcp-events-grants.spec.ts",

‎e2e/tests/mcp-events-app-removal.spec.ts‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
/**
2+
* Removing an app removes its event subscriptions. A client that subscribes again by the same
3+
* name after the app was re-created under that name gets a subscription bound to the new app,
4+
* with the same id, and receives the new app's events. A caller who can no longer see a
5+
* subscription's app is told its event does not exist.
6+
*/
7+
import { expect, layer } from "@effect/vitest";
8+
import { Effect, Schema } from "effect";
9+
import { randomUUID } from "node:crypto";
10+
import { scenarios } from "../test-plan.ts";
11+
import { Api, body } from "../support/api.ts";
12+
import { Actors } from "../support/actors.ts";
13+
import { HostedLive, withHostedCase } from "../support/case.ts";
14+
import { App } from "../support/contracts.ts";
15+
import { Granted, emitterFiles, eventFixtures, secrets } from "../support/events.ts";
16+
17+
layer(HostedLive, { excludeTestServices: true })("MCP event app removal", (it) => {
18+
it.effect(
19+
scenarios.mcpEventsAppRemoval.title,
20+
(context) =>
21+
withHostedCase(
22+
context,
23+
Effect.gen(function* () {
24+
const api = yield* Api,
25+
actors = yield* Actors;
26+
const {
27+
prefix,
28+
suffix,
29+
emitter,
30+
callbackUrl,
31+
awaitDeliveries,
32+
events,
33+
emit,
34+
rpcAs,
35+
ok,
36+
createKey,
37+
name,
38+
} = yield* eventFixtures;
39+
const key = yield* createKey(`Event app removal ${suffix}`);
40+
const subscribe = () =>
41+
rpcAs(key.key, "events/subscribe", {
42+
name,
43+
arguments: {},
44+
delivery: { mode: "webhook", url: callbackUrl, secret: secrets[0] },
45+
ttlMs: 60 * 60_000,
46+
});
47+
48+
const first = yield* ok(Granted, yield* subscribe());
49+
const before = yield* emit("acme/widgets", 1);
50+
yield* awaitDeliveries((all) =>
51+
events(all).some(
52+
(delivery) => delivery.id === before && delivery.subscription === first.id,
53+
),
54+
);
55+
56+
const removed = yield* api.request(
57+
actors.owner,
58+
"DELETE",
59+
`${prefix}/apps/${emitter.id}`,
60+
);
61+
expect(removed.status, JSON.stringify(removed.body)).toBe(200);
62+
63+
// The same name gives the same slug, so the event name the client holds resolves again.
64+
const created: { app?: string } = {};
65+
yield* Effect.addFinalizer(() =>
66+
Effect.gen(function* () {
67+
if (created.app)
68+
yield* api.request(actors.owner, "DELETE", `${prefix}/apps/${created.app}`);
69+
}).pipe(Effect.orDie),
70+
);
71+
const deployed = yield* api.request(actors.owner, "POST", `${prefix}/apps/deploy`, {
72+
name: `Issue events ${suffix}`,
73+
files: emitterFiles,
74+
});
75+
expect(deployed.status, JSON.stringify(deployed.body)).toBe(200);
76+
const fresh = yield* body(App, deployed);
77+
created.app = fresh.id;
78+
expect(fresh.id).not.toBe(emitter.id);
79+
expect(fresh.slug).toBe(emitter.slug);
80+
81+
const second = yield* ok(Granted, yield* subscribe());
82+
expect(second.id).toBe(first.id);
83+
84+
const after = randomUUID();
85+
const emitted = yield* api.request(
86+
actors.owner,
87+
"POST",
88+
`${prefix}/apps/${fresh.id}/tools/call`,
89+
{
90+
tool: "open",
91+
kind: "mutation",
92+
input: { repo: "acme/widgets", title: "Issue 2", number: 2, id: after },
93+
},
94+
);
95+
expect(emitted.status, JSON.stringify(emitted.body)).toBe(200);
96+
yield* awaitDeliveries((all) =>
97+
events(all).some(
98+
(delivery) => delivery.id === after && delivery.subscription === second.id,
99+
),
100+
);
101+
102+
yield* ok(
103+
Schema.Struct({}),
104+
yield* rpcAs(key.key, "events/unsubscribe", {
105+
name,
106+
arguments: {},
107+
delivery: { mode: "webhook", url: callbackUrl },
108+
}),
109+
);
110+
}),
111+
),
112+
{ timeout: 240_000 },
113+
);
114+
115+
it.effect(
116+
scenarios.mcpEventsAppHidden.title,
117+
(context) =>
118+
withHostedCase(
119+
context,
120+
Effect.gen(function* () {
121+
const api = yield* Api,
122+
actors = yield* Actors;
123+
const { prefix, suffix, emitter, callbackUrl, rpcAs, ok, refused, name } =
124+
yield* eventFixtures;
125+
const key = yield* body(
126+
Schema.Struct({ id: Schema.String, key: Schema.RedactedFromValue(Schema.String) }),
127+
yield* api.request(actors.admin, "POST", "/api/auth/api-key/create", {
128+
name: `Event app hidden ${suffix}`,
129+
}),
130+
);
131+
yield* Effect.addFinalizer(() =>
132+
api
133+
.request(actors.admin, "POST", "/api/auth/api-key/delete", { keyId: key.id })
134+
.pipe(Effect.ignore),
135+
);
136+
const subscribe = () =>
137+
rpcAs(key.key, "events/subscribe", {
138+
name,
139+
arguments: {},
140+
delivery: { mode: "webhook", url: callbackUrl, secret: secrets[0] },
141+
ttlMs: 60 * 60_000,
142+
});
143+
// Apps start private to their creator; sharing lets the administrator subscribe.
144+
const share = (kind: "everyone" | "private") =>
145+
Effect.gen(function* () {
146+
const access = yield* body(
147+
Schema.Struct({ revision: Schema.String }),
148+
yield* api.request(actors.owner, "GET", `${prefix}/apps/${emitter.id}/access`),
149+
);
150+
const updated = yield* api.request(
151+
actors.owner,
152+
"PATCH",
153+
`${prefix}/apps/${emitter.id}/access`,
154+
{ revision: access.revision, audience: { kind } },
155+
);
156+
expect(updated.status, JSON.stringify(updated.body)).toBe(200);
157+
});
158+
yield* share("everyone");
159+
yield* ok(Granted, yield* subscribe());
160+
yield* share("private");
161+
162+
expect(yield* refused(yield* subscribe())).toMatchObject({
163+
code: -32011,
164+
data: { kind: "event" },
165+
});
166+
}),
167+
),
168+
{ timeout: 240_000 },
169+
);
170+
});

‎packages/sdk/src/implementation/apps.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,30 @@ export const makeApps = (
653653
);
654654

655655
yield* query(() => tx.deleteMany("webhooks", { where: (b) => b("app", "=", app.id) }));
656+
const subscriptions = yield* query(() =>
657+
tx.findMany("eventSubscriptions", {
658+
select: ["id"],
659+
where: (b) => b("app", "=", app.id),
660+
}),
661+
);
662+
// A subscription's ID omits the app, so one left behind would pin its event name
663+
// to this app after another app takes the name.
664+
if (subscriptions.length > 0) {
665+
yield* query(() =>
666+
tx.deleteMany("eventDeliveries", {
667+
where: (b) =>
668+
b(
669+
"subscription",
670+
"in",
671+
subscriptions.map((subscription) => subscription.id),
672+
),
673+
}),
674+
);
675+
yield* query(() =>
676+
tx.deleteMany("eventSubscriptions", { where: (b) => b("app", "=", app.id) }),
677+
);
678+
}
679+
yield* query(() => tx.deleteMany("events", { where: (b) => b("app", "=", app.id) }));
656680
yield* query(() =>
657681
tx.deleteMany("appRecords", { where: (b) => b("app", "=", app.id) }),
658682
);

‎packages/sdk/src/implementation/owners.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,30 @@ export const makeOwners = (db: Query): Executor["owners"] => ({
123123
yield* query(() => tx.deleteMany("schedules", { where: (b) => b("owner", "=", owner) }));
124124
// Profiles run as the app owner; delete by owner for the same reason as schedules.
125125
yield* query(() => tx.deleteMany("profiles", { where: (b) => b("owner", "=", owner) }));
126+
// Event subscriptions too, so a subscription whose app row already went stays reachable.
127+
const subscriptions = yield* query(() =>
128+
tx.findMany("eventSubscriptions", {
129+
select: ["id"],
130+
where: (b) => b("owner", "=", owner),
131+
}),
132+
);
133+
if (subscriptions.length > 0) {
134+
yield* query(() =>
135+
tx.deleteMany("eventDeliveries", {
136+
where: (b) =>
137+
b(
138+
"subscription",
139+
"in",
140+
subscriptions.map((subscription) => subscription.id),
141+
),
142+
}),
143+
);
144+
yield* query(() =>
145+
tx.deleteMany("eventSubscriptions", { where: (b) => b("owner", "=", owner) }),
146+
);
147+
}
126148
if (appIds.length > 0) {
149+
yield* query(() => tx.deleteMany("events", { where: (b) => b("app", "in", appIds) }));
127150
yield* query(() => tx.deleteMany("appRecords", { where: (b) => b("app", "in", appIds) }));
128151
yield* query(() =>
129152
tx.deleteMany("accountChecks", { where: (b) => b("app", "in", appIds) }),

0 commit comments

Comments
 (0)