Skip to content

Commit bb25c49

Browse files
committed
fix(runtime): a package's jobs stop on uninstall and when a reinstall drops them
The binder's job half records which jobs each app scheduled and cancels the ones a new version no longer schedules; the runtime.package-jobs uninstall cleanup, registered on the protocol's registry when a package's jobs are first scheduled, cancels an uninstalled package's jobs on every uninstall door. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
1 parent 37c4727 commit bb25c49

3 files changed

Lines changed: 374 additions & 24 deletions

File tree

‎packages/cloud-connection/src/marketplace-install-local-jobs.test.ts‎

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ const BODY_JOB = {
5656
const HANDLER_JOB = { name: 'jobs_app_tick_handler', schedule: INTERVAL, handler: 'tick' };
5757

5858
/** The compiled-artifact shape `os build` writes and `os package install` sends. */
59-
function artifact(jobs: unknown[] | undefined) {
59+
function artifact(jobs: unknown[] | undefined, id: string = APP_ID) {
6060
return {
61-
manifest: { id: APP_ID, namespace: 'jobs_app', version: '0.1.0', type: 'app', name: 'Jobs App' },
61+
manifest: { id, namespace: 'jobs_app', version: '0.1.0', type: 'app', name: 'Jobs App' },
6262
objects: [{ name: TICK, label: 'Tick', fields: { name: { type: 'text', label: 'Name' } } }],
6363
...(jobs ? { jobs } : {}),
6464
};
@@ -86,18 +86,53 @@ function recordingEngine() {
8686
/** `IJobService` as state: what was handed to `schedule`, by job name. */
8787
function recordingJobService() {
8888
const scheduled = new Map<string, { run: (c: any) => Promise<unknown>; options: unknown }>();
89+
const cancels: string[] = [];
8990
return {
9091
scheduled,
92+
cancels,
9193
svc: {
9294
schedule: vi.fn(async (name: string, _schedule: unknown, run: (c: any) => Promise<unknown>, options?: unknown) => {
9395
scheduled.set(name, { run, options });
9496
}),
95-
cancel: async () => undefined,
97+
// The adapters' own semantics: `cancel` stops the job and forgets it.
98+
cancel: async (name: string) => { cancels.push(name); scheduled.delete(name); },
9699
trigger: async () => undefined,
97100
},
98101
};
99102
}
100103

104+
/**
105+
* The protocol's uninstall-cleanup registry, as its two verbs behave
106+
* (`packages/metadata-protocol`, #21490): one cleanup per name, and ONE runner
107+
* that calls every registered cleanup with the package id and reports each
108+
* outcome. The runner itself is pinned where it lives; this models it so the
109+
* door's `DELETE` reaches what the binder registered.
110+
*/
111+
function registryProtocol() {
112+
const cleanups = new Map<string, (args: { packageId: string }) => Promise<{ success: boolean; removed: number; error?: string }>>();
113+
return {
114+
registerUninstallCleanup: (name: string, cleanup: any) => { cleanups.set(name, cleanup); },
115+
runUninstallCleanups: async (request: { packageId: string }) => {
116+
const out: unknown[] = [];
117+
for (const [name, cleanup] of cleanups) out.push({ name, ...(await cleanup({ packageId: request.packageId })) });
118+
return out;
119+
},
120+
};
121+
}
122+
123+
function makeDeleteC(manifestId: string) {
124+
const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 }));
125+
return {
126+
req: {
127+
url: `http://localhost:3000/api/v1/marketplace/install-local/${manifestId}`,
128+
raw: new Request('http://localhost:3000/x'),
129+
json: async () => ({}),
130+
param: (name: string) => (name === 'manifestId' ? manifestId : undefined),
131+
},
132+
json,
133+
};
134+
}
135+
101136
/**
102137
* The package ids handed to `manifest.register` — the plugin also registers its
103138
* own Setup nav bundle at `kernel:ready`, which is not the package under test.
@@ -158,6 +193,7 @@ async function bootPlugin() {
158193
auth: installerAuthService(),
159194
objectql: withInstallerGrants(rec.engine),
160195
job: jobs.svc,
196+
protocol: registryProtocol(),
161197
};
162198
const ctx = {
163199
hook: (e: string, h: any) => hooks.set(e, h),
@@ -174,7 +210,9 @@ async function bootPlugin() {
174210
await hooks.get('kernel:ready')?.();
175211
const install = async (bundle: unknown) =>
176212
rawApp.routes.get('POST /api/v1/marketplace/install-local')!(makeC({ manifest: bundle }));
177-
return { install, rec, jobs, register, logger };
213+
const uninstall = async (manifestId: string) =>
214+
rawApp.routes.get('DELETE /api/v1/marketplace/install-local/:manifestId')!(makeDeleteC(manifestId));
215+
return { install, uninstall, rec, jobs, register, logger };
178216
}
179217

180218
describe('#21489: install-local schedules an installed package’s job bodies', () => {
@@ -215,6 +253,36 @@ describe('#21489: install-local schedules an installed package’s job bodies',
215253
});
216254
});
217255

256+
describe('#21489: an uninstalled or replaced package’s jobs STOP', () => {
257+
const OTHER_ID = 'com.example.otherjobs';
258+
const OTHER_JOB = { ...BODY_JOB, name: 'other_jobs_tick_body' };
259+
260+
it('DELETE cancels the uninstalled package’s jobs through the uninstall cleanup — and no other package’s', async () => {
261+
const { install, uninstall, jobs } = await bootPlugin();
262+
expect((await install(artifact([BODY_JOB]))).status).toBe(200);
263+
expect((await install(artifact([OTHER_JOB], OTHER_ID))).status).toBe(200);
264+
265+
const res = await uninstall(APP_ID);
266+
267+
expect(res.status, JSON.stringify(res.payload)).toBe(200);
268+
expect(jobs.cancels).toEqual([BODY_JOB.name]);
269+
expect([...jobs.scheduled.keys()], 'the control package’s job keeps running').toEqual([OTHER_JOB.name]);
270+
expect(res.payload.data.cleanups).toContainEqual({ name: 'runtime.package-jobs', success: true, removed: 1 });
271+
});
272+
273+
it('a reinstall whose new version DROPS a job cancels it, and keeps the job it still declares', async () => {
274+
const { install, jobs } = await bootPlugin();
275+
const KEPT = { ...BODY_JOB, name: 'jobs_app_kept' };
276+
const GONE = { ...BODY_JOB, name: 'jobs_app_gone' };
277+
expect((await install(artifact([KEPT, GONE]))).status).toBe(200);
278+
279+
expect((await install(artifact([KEPT]))).status).toBe(200);
280+
281+
expect(jobs.cancels).toEqual([GONE.name]);
282+
expect([...jobs.scheduled.keys()]).toEqual([KEPT.name]);
283+
});
284+
});
285+
218286
describe('#21489: install-local refuses an enabled job with no body', () => {
219287
it('answers 422 VALIDATION_ERROR naming the job, its handler and both remedies — and changes nothing', async () => {
220288
const { install, rec, jobs, register } = await bootPlugin();

‎packages/runtime/src/app-artifact-handlers.jobs.test.ts‎

Lines changed: 133 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
import { describe, it, expect, vi } from 'vitest';
2727
import type { PluginContext } from '@objectstack/core';
28-
import { scheduleAppArtifactJobs, collectJobsWithoutBody } from './app-artifact-handlers.js';
28+
import { scheduleAppArtifactJobs, collectJobsWithoutBody, PACKAGE_JOBS_UNINSTALL_CLEANUP } from './app-artifact-handlers.js';
2929
import { jobBodyRunnerFactory } from './sandbox/body-runner.js';
3030
import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js';
3131
import { AppPlugin } from './app-plugin.js';
@@ -64,36 +64,62 @@ function recordingEngine() {
6464
};
6565
}
6666

67-
/** `IJobService` as state: what was handed to `schedule`, by job name. */
68-
function recordingJobService() {
67+
/**
68+
* `IJobService` as state: what is scheduled right now, by job name — `schedule`
69+
* replaces by name and `cancel` removes, the adapters' own semantics — plus
70+
* every cancel in order. `failCancel` names a job whose cancel throws.
71+
*/
72+
function recordingJobService(opts: { failCancel?: string } = {}) {
6973
const scheduled = new Map<string, { schedule: unknown; run: (c: any) => Promise<unknown>; options: unknown }>();
74+
const cancels: string[] = [];
7075
return {
7176
scheduled,
77+
cancels,
7278
svc: {
7379
schedule: async (name: string, schedule: unknown, run: (c: any) => Promise<unknown>, options?: unknown) => {
7480
scheduled.set(name, { schedule, run, options });
7581
},
76-
cancel: async () => undefined,
82+
cancel: async (name: string) => {
83+
if (name === opts.failCancel) throw new Error(`cannot cancel ${name}`);
84+
cancels.push(name);
85+
scheduled.delete(name);
86+
},
7787
trigger: async () => undefined,
7888
},
7989
};
8090
}
8191

82-
function harness() {
92+
/** The protocol's uninstall-cleanup registry as state — the two verbs the binder and the doors use. */
93+
function recordingProtocol() {
94+
const cleanups = new Map<string, (args: { packageId: string }) => Promise<{ success: boolean; removed: number; error?: string }>>();
95+
const registrations: string[] = [];
96+
return {
97+
cleanups,
98+
registrations,
99+
protocol: {
100+
registerUninstallCleanup: (name: string, cleanup: any) => { registrations.push(name); cleanups.set(name, cleanup); },
101+
},
102+
};
103+
}
104+
105+
function harness(opts: { failCancel?: string; withProtocol?: boolean } = {}) {
83106
const engine = recordingEngine();
84-
const jobs = recordingJobService();
107+
const jobs = recordingJobService({ failCancel: opts.failCancel });
108+
const reg = recordingProtocol();
85109
const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() };
86110
const ctx = {
87111
logger,
88112
getService: (name: string) => {
89113
if (name === 'job') return jobs.svc;
114+
if (name === 'protocol' && opts.withProtocol) return reg.protocol;
90115
throw new Error(`no ${name}`);
91116
},
92117
} as unknown as PluginContext;
93-
const schedule = (bundle: unknown) =>
94-
scheduleAppArtifactJobs(ctx, bundle, { appId: APP_ID, ql: engine.ql as any, source: 'Test' });
118+
const schedule = (bundle: unknown, appId: string = APP_ID) =>
119+
scheduleAppArtifactJobs(ctx, bundle, { appId, ql: engine.ql as any, source: 'Test' });
95120
const warned = () => logger.warn.mock.calls.map((c) => String(c[0]));
96-
return { engine, jobs, logger, ctx, schedule, warned };
121+
const errored = () => logger.error.mock.calls.map((c) => String(c[0]));
122+
return { engine, jobs, reg, logger, ctx, schedule, warned, errored };
97123
}
98124

99125
/** A flattened JSON package, as install-local holds it: no `functions`. */
@@ -240,7 +266,7 @@ describe('#21489: scheduleAppArtifactJobs — handler jobs and the door-wide gat
240266
{ name: 'off_handler', schedule: INTERVAL, handler: 'tick', enabled: false },
241267
]));
242268

243-
expect(out).toEqual({ bodies: [], handlers: [], notScheduled: [], failed: [] });
269+
expect(out).toEqual({ bodies: [], handlers: [], notScheduled: [], failed: [], cancelled: [] });
244270
expect(h.jobs.scheduled.size).toBe(0);
245271
});
246272

@@ -258,6 +284,103 @@ describe('#21489: scheduleAppArtifactJobs — handler jobs and the door-wide gat
258284
});
259285
});
260286

287+
describe('#21489: re-scheduling replaces — a job the new version does not schedule is CANCELLED', () => {
288+
const job = (name: string) => ({ name, schedule: INTERVAL, body: WRITE_BODY });
289+
290+
it('a reinstall that DROPS a job cancels it, and keeps the one it still declares', async () => {
291+
const h = harness();
292+
await h.schedule(pkg([job('kept_job'), job('gone_job')]));
293+
294+
const out = await h.schedule(pkg([job('kept_job')]));
295+
296+
expect(out.cancelled).toEqual(['gone_job']);
297+
expect(h.jobs.cancels).toEqual(['gone_job']);
298+
expect([...h.jobs.scheduled.keys()]).toEqual(['kept_job']);
299+
});
300+
301+
it('a version that disables a job, or declares no jobs at all, cancels what it no longer runs', async () => {
302+
const h = harness();
303+
await h.schedule(pkg([job('a_job'), job('b_job')]));
304+
305+
const disabled = await h.schedule(pkg([job('a_job'), { ...job('b_job'), enabled: false }]));
306+
expect(disabled.cancelled).toEqual(['b_job']);
307+
308+
const none = await h.schedule(pkg([]));
309+
expect(none.cancelled).toEqual(['a_job']);
310+
expect(h.jobs.scheduled.size).toBe(0);
311+
});
312+
313+
it("another app's jobs are never cancelled — not even one that took over a name this app once scheduled", async () => {
314+
const h = harness();
315+
await h.schedule(pkg([job('shared_name'), job('mine_only')]), APP_ID);
316+
await h.schedule(pkg([job('shared_name'), job('theirs_only')]), 'com.example.other');
317+
318+
// This app's next version drops both: only its own remaining job stops.
319+
const out = await h.schedule(pkg([]), APP_ID);
320+
321+
expect(out.cancelled).toEqual(['mine_only']);
322+
expect([...h.jobs.scheduled.keys()].sort()).toEqual(['shared_name', 'theirs_only']);
323+
});
324+
325+
it('a cancel that throws is said at error, and the job stays on the record for the next attempt', async () => {
326+
const h = harness({ failCancel: 'stuck_job' });
327+
await h.schedule(pkg([job('stuck_job')]));
328+
329+
const first = await h.schedule(pkg([]));
330+
expect(first.cancelled).toEqual([]);
331+
expect(h.errored().some((m) => m.includes('could NOT be cancelled'))).toBe(true);
332+
});
333+
});
334+
335+
describe('#21489: the uninstall cleanup cancels the uninstalled package\'s jobs', () => {
336+
const job = (name: string) => ({ name, schedule: INTERVAL, body: WRITE_BODY });
337+
338+
it('is registered once per protocol, as runtime.package-jobs, when a package\'s jobs are scheduled', async () => {
339+
const h = harness({ withProtocol: true });
340+
341+
await h.schedule(pkg([job('a_job')]), APP_ID);
342+
await h.schedule(pkg([job('b_job')]), 'com.example.other');
343+
344+
expect(h.reg.registrations).toEqual([PACKAGE_JOBS_UNINSTALL_CLEANUP]);
345+
expect(PACKAGE_JOBS_UNINSTALL_CLEANUP).toBe('runtime.package-jobs');
346+
});
347+
348+
it("cancels every job of the uninstalled package and none of another package's", async () => {
349+
const h = harness({ withProtocol: true });
350+
await h.schedule(pkg([job('a_job'), job('a_other')]), APP_ID);
351+
await h.schedule(pkg([job('b_job')]), 'com.example.other');
352+
const cleanup = h.reg.cleanups.get(PACKAGE_JOBS_UNINSTALL_CLEANUP)!;
353+
354+
const result = await cleanup({ packageId: APP_ID });
355+
356+
expect(result).toEqual({ success: true, removed: 2 });
357+
expect(h.jobs.cancels.sort()).toEqual(['a_job', 'a_other']);
358+
expect([...h.jobs.scheduled.keys()]).toEqual(['b_job']);
359+
// A second uninstall of the same package has nothing left to cancel.
360+
await expect(cleanup({ packageId: APP_ID })).resolves.toEqual({ success: true, removed: 0 });
361+
});
362+
363+
it('a package that scheduled nothing is a no-op', async () => {
364+
const h = harness({ withProtocol: true });
365+
await h.schedule(pkg([job('a_job')]), APP_ID);
366+
367+
await expect(h.reg.cleanups.get(PACKAGE_JOBS_UNINSTALL_CLEANUP)!({ packageId: 'com.example.never' }))
368+
.resolves.toEqual({ success: true, removed: 0 });
369+
expect(h.jobs.cancels).toEqual([]);
370+
});
371+
372+
it('a job it could not cancel is an outcome, never a throw — success:false naming the job', async () => {
373+
const h = harness({ withProtocol: true, failCancel: 'stuck_job' });
374+
await h.schedule(pkg([job('stuck_job'), job('fine_job')]), APP_ID);
375+
376+
const result = await h.reg.cleanups.get(PACKAGE_JOBS_UNINSTALL_CLEANUP)!({ packageId: APP_ID });
377+
378+
expect(result.success).toBe(false);
379+
expect(result.removed).toBe(1);
380+
expect(result.error).toContain('stuck_job');
381+
});
382+
});
383+
261384
describe('#21489: the sandbox job origin', () => {
262385
it("a job body with no timeoutMs gets the runner's JOB default — not the hook's, not the action's", async () => {
263386
const runner = new QuickJSScriptRunner({ jobTimeoutMs: 30, hookTimeoutMs: 5_000, actionTimeoutMs: 5_000 });

0 commit comments

Comments
 (0)