Skip to content

Commit 4320abb

Browse files
committed
fix(runtime,cloud-connection)!: install-local refuses an enabled job whose pull does not bind
collectJobsWithoutBody judges a pull job by the binder's own judgeJobPull and names an unbindable one with its pullRefusal; describeUnrunnable gains the pull clause. The door answers 422 VALIDATION_ERROR. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GiHYRmLSNWTfbX9gVQkpz
1 parent fd2a524 commit 4320abb

5 files changed

Lines changed: 269 additions & 51 deletions

File tree

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

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@
2424
* it was found — nothing registered, persisted or scheduled;
2525
* - #21585: so does an enabled job whose `body` the declaration refuses (an
2626
* expression body, a `body.timeoutMs`), naming the refused key;
27+
* - so does an enabled job whose `pull` does not bind (a mapping the package
28+
* does not declare, one with no `connectorSource`), naming the refusal the
29+
* binder's own `judgeJobPull` gives; a pull naming a declared mapping
30+
* installs and is scheduled, a DISABLED unbindable one installs, and an
31+
* entry an earlier build persisted rehydrates with its unbindable pull job
32+
* withheld and warned by name;
2733
* - a package without jobs, and one whose handler-only job is DISABLED,
2834
* install unchanged.
2935
*
@@ -190,12 +196,20 @@ async function bootPlugin() {
190196
const rawApp = makeRawApp();
191197
const hooks = new Map<string, any>();
192198
const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() };
199+
// The `automation` service a pull job's run calls (`IAutomationService.pullConnectorSource`).
200+
const automation = {
201+
pullConnectorSource: vi.fn(async (request: { mapping: string }) => ({
202+
mapping: request.mapping, targetObject: 'order', connector: 'orders_api', action: 'request', pulled: 0,
203+
summary: { total: 0, processed: 0, created: 0, updated: 0, skipped: 0, errors: 0, ok: 0, cancelled: false },
204+
})),
205+
};
193206
const services: Record<string, unknown> = {
194207
manifest: { register },
195208
auth: installerAuthService(),
196209
objectql: withInstallerGrants(rec.engine),
197210
job: jobs.svc,
198211
protocol: registryProtocol(),
212+
automation,
199213
};
200214
const ctx = {
201215
hook: (e: string, h: any) => hooks.set(e, h),
@@ -214,7 +228,7 @@ async function bootPlugin() {
214228
rawApp.routes.get('POST /api/v1/marketplace/install-local')!(makeC({ manifest: bundle }));
215229
const uninstall = async (manifestId: string) =>
216230
rawApp.routes.get('DELETE /api/v1/marketplace/install-local/:manifestId')!(makeDeleteC(manifestId));
217-
return { install, uninstall, rec, jobs, register, logger };
231+
return { install, uninstall, rec, jobs, register, logger, automation };
218232
}
219233

220234
describe('#21489: install-local schedules an installed package’s job bodies', () => {
@@ -378,3 +392,109 @@ describe('#21489: install-local refuses an enabled job with no body', () => {
378392
expect(jobs.svc.schedule).not.toHaveBeenCalled();
379393
});
380394
});
395+
396+
describe('install-local refuses an enabled job whose pull does not bind, by the binder\'s own judgeJobPull', () => {
397+
const MAPPING = {
398+
name: 'orders_pull',
399+
targetObject: TICK,
400+
fieldMapping: [{ source: 'id', target: 'name' }],
401+
mode: 'upsert',
402+
upsertKey: ['name'],
403+
connectorSource: { connector: 'orders_api', action: 'request' },
404+
};
405+
const { connectorSource: _dropped, ...IMPORT_ONLY } = MAPPING;
406+
const PULL_JOB = { name: 'jobs_app_pull', schedule: INTERVAL, pull: { mapping: 'orders_pull' } };
407+
const UNDECLARED = { ...PULL_JOB, name: 'jobs_app_pull_typo', pull: { mapping: 'orders_pul' } };
408+
409+
/** The compiled-artifact shape, carrying `mappings` beside `jobs`. */
410+
const withMappings = (jobs: unknown[], mappings: unknown[] = [MAPPING]) => ({ ...artifact(jobs), mappings });
411+
412+
it('a pull naming a mapping the package does not declare answers 422 VALIDATION_ERROR naming the job and the refusal — and changes nothing', async () => {
413+
const { install, jobs, register, automation } = await bootPlugin();
414+
415+
const res = await install(withMappings([BODY_JOB, UNDECLARED]));
416+
417+
expect(res.status).toBe(422);
418+
expect(res.payload.success).toBe(false);
419+
expect(res.payload.error.code).toBe('VALIDATION_ERROR');
420+
const message: string = res.payload.error.message;
421+
expect(message).toContain(`its enabled job '${UNDECLARED.name}' (pull.mapping: this artifact declares no mapping 'orders_pul'`);
422+
expect(message).toContain('has a `pull` that does not bind');
423+
expect(message).toContain('os validate');
424+
// The pull's own clause, never the no-`body` one: a `body` beside a `pull` is refused by the declaration.
425+
expect(message).not.toMatch(/give the job a `body`/i);
426+
// The runtime is left exactly as it was found.
427+
expect(registered(register), 'a refused package must not be registered').toEqual([]);
428+
expect(new LocalManifestSource(dir).read(APP_ID).entry, 'nor persisted').toBeNull();
429+
expect(jobs.svc.schedule, 'nor any of its jobs scheduled — not even its body job').not.toHaveBeenCalled();
430+
expect(automation.pullConnectorSource).not.toHaveBeenCalled();
431+
});
432+
433+
it('a pull whose mapping declares no connectorSource is refused the same way', async () => {
434+
const { install, jobs, register } = await bootPlugin();
435+
436+
const res = await install(withMappings([PULL_JOB], [IMPORT_ONLY]));
437+
438+
expect(res.status).toBe(422);
439+
expect(res.payload.error.code).toBe('VALIDATION_ERROR');
440+
expect(res.payload.error.message).toContain(`its enabled job '${PULL_JOB.name}' (pull.mapping: mapping 'orders_pull' declares no connectorSource`);
441+
expect(registered(register)).toEqual([]);
442+
expect(jobs.svc.schedule).not.toHaveBeenCalled();
443+
});
444+
445+
it('one answer names every kind the door cannot run — the unbindable pull beside a job with no body', async () => {
446+
const { install } = await bootPlugin();
447+
448+
const res = await install(withMappings([HANDLER_JOB, UNDECLARED]));
449+
450+
expect(res.status).toBe(422);
451+
const message: string = res.payload.error.message;
452+
expect(message).toContain(`'${HANDLER_JOB.name}' (handler 'tick') has no \`body\``);
453+
expect(message).toContain(`'${UNDECLARED.name}' (pull.mapping: `);
454+
});
455+
456+
it('a DISABLED pull job naming an undeclared mapping does not block the install, and is not scheduled', async () => {
457+
const { install, jobs, register } = await bootPlugin();
458+
459+
const res = await install(withMappings([{ ...UNDECLARED, enabled: false }]));
460+
461+
expect(res.status, JSON.stringify(res.payload)).toBe(200);
462+
expect(registered(register)).toEqual([APP_ID]);
463+
expect(jobs.svc.schedule).not.toHaveBeenCalled();
464+
});
465+
466+
it('control: a pull naming a declared mapping with a connectorSource installs, is scheduled, and a run pulls that mapping', async () => {
467+
const { install, jobs, automation } = await bootPlugin();
468+
469+
const res = await install(withMappings([PULL_JOB]));
470+
471+
expect(res.status, JSON.stringify(res.payload)).toBe(200);
472+
expect([...jobs.scheduled.keys()]).toEqual([PULL_JOB.name]);
473+
await jobs.scheduled.get(PULL_JOB.name)!.run({ jobId: PULL_JOB.name });
474+
expect(automation.pullConnectorSource).toHaveBeenCalledTimes(1);
475+
expect(automation.pullConnectorSource.mock.calls[0][0]).toMatchObject({ mapping: 'orders_pull' });
476+
});
477+
478+
it('rehydrate — an entry an earlier build persisted with an unbindable pull job: the job is withheld and warned by name, the bindable one scheduled', async () => {
479+
const { manifest: meta, ...sections } = withMappings([PULL_JOB, UNDECLARED]);
480+
new LocalManifestSource(dir).write({
481+
packageId: APP_ID,
482+
versionId: 'local',
483+
manifestId: APP_ID,
484+
version: '0.1.0',
485+
manifest: { ...meta, ...sections },
486+
installedAt: '2026-01-01T00:00:00.000Z',
487+
installedBy: 'admin',
488+
withSampleData: false,
489+
});
490+
491+
const { jobs, logger, register } = await bootPlugin();
492+
493+
expect(registered(register), 'the entry still rehydrates').toEqual([APP_ID]);
494+
expect([...jobs.scheduled.keys()]).toEqual([PULL_JOB.name]);
495+
const warned = logger.warn.mock.calls.find(([message, meta]) =>
496+
String(message).includes('NOT scheduled') && (meta as { job?: string } | undefined)?.job === UNDECLARED.name);
497+
expect(warned, 'no warn names the withheld pull job').toBeDefined();
498+
expect(String(warned![0])).toContain("pull.mapping: this artifact declares no mapping 'orders_pul'");
499+
});
500+
});

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

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -166,21 +166,22 @@ const INSTALL_LOCAL_CAPABILITY = 'manage_metadata';
166166

167167
/**
168168
* [#21489, #21585] The refusal of a package that declares code this door cannot
169-
* run — an enabled job with no `body` or with a `body` the declaration refuses,
170-
* a hook with no `body`: `VALIDATION_ERROR` / 422.
169+
* run — an enabled job with no `body`, with a `body` the declaration refuses or
170+
* with a `pull` that does not bind, a hook with no `body`: `VALIDATION_ERROR` /
171+
* 422.
171172
*
172173
* The code is the standard catalog's input-validation member. The condition is
173174
* that the install payload fails this door's acceptance rule — every enabled
174-
* job carries a `body` that binds, and every hook carries a `body` — and the
175-
* ledger's admission rule sends a generic validation condition to the standard
176-
* member rather than to a registered synonym (`error-code-ledger.zod.ts`,
175+
* job carries a `body` or a `pull` that binds, and every hook carries a `body`
176+
* — and the ledger's admission rule sends a generic validation condition to the
177+
* standard member rather than to a registered synonym (`error-code-ledger.zod.ts`,
177178
* "Registering a new code"). What the author does instead is the prescription
178179
* the message carries. `PLUGIN_MANIFEST_INVALID` is deliberately not it: that
179180
* code answers the manifest's identity at this door, and a handler-form job or
180181
* hook is a valid manifest — `os validate` passes it and `os start --artifact`
181-
* runs it. One acceptance rule answers one code, so the off-spec job `body`
182-
* (which `os validate` does refuse) is answered by the same refusal as the rest
183-
* of the rule rather than splitting it.
182+
* runs it. One acceptance rule answers one code, so the off-spec job `body` and
183+
* the unbindable `pull` (which `os validate` does refuse) are answered by the
184+
* same refusal as the rest of the rule rather than splitting it.
184185
*
185186
* The status is 422, not the 400 / 502 split this door uses for an invalid
186187
* manifest id: the package is well-formed JSON this door cannot process, which
@@ -193,7 +194,7 @@ const UNRUNNABLE_REFUSAL_STATUS = 422;
193194

194195
/** What this door cannot run in a package, as the runtime binder judges it. */
195196
interface UnrunnableCode {
196-
jobs: ReadonlyArray<{ name: string; handler?: string; bodyRefusal?: string }>;
197+
jobs: ReadonlyArray<{ name: string; handler?: string; bodyRefusal?: string; pullRefusal?: string }>;
197198
hooks: ReadonlyArray<{ name: string; handler?: string }>;
198199
}
199200

@@ -207,14 +208,16 @@ const capitalize = (text: string) => text.charAt(0).toUpperCase() + text.slice(1
207208
/**
208209
* The refusal sentence: everything the door cannot run, why, and the remedies,
209210
* one clause per kind — a job with no `body`, a job whose `body` the
210-
* declaration refuses, a hook with no `body` — in one answer, so the author
211-
* fixes them all in one pass. Each names the item and the function its
212-
* `handler` declares (or the declaration's refusal of its `body`).
211+
* declaration refuses, a job whose `pull` does not bind, a hook with no `body`
212+
* — in one answer, so the author fixes them all in one pass. Each names the
213+
* item and the function its `handler` declares (or the refusal of its `body` or
214+
* its `pull`).
213215
*/
214216
function describeUnrunnable(manifestId: string, what: UnrunnableCode): string {
215217
const clauses: string[] = [];
216-
const jobsWithoutBody = what.jobs.filter((j) => j.bodyRefusal === undefined);
217-
const jobsWithBadBody = what.jobs.filter((j) => j.bodyRefusal !== undefined);
218+
const jobsWithBadPull = what.jobs.filter((j) => j.pullRefusal !== undefined);
219+
const jobsWithoutBody = what.jobs.filter((j) => j.pullRefusal === undefined && j.bodyRefusal === undefined);
220+
const jobsWithBadBody = what.jobs.filter((j) => j.pullRefusal === undefined && j.bodyRefusal !== undefined);
218221
if (jobsWithoutBody.length > 0) {
219222
const one = jobsWithoutBody.length === 1;
220223
clauses.push(
@@ -236,6 +239,16 @@ function describeUnrunnable(manifestId: string, what: UnrunnableCode): string {
236239
+ "body; the job's time limit is the job's own `timeoutMs`) — `os validate` reports the same refusal.",
237240
);
238241
}
242+
if (jobsWithBadPull.length > 0) {
243+
const one = jobsWithBadPull.length === 1;
244+
const list = jobsWithBadPull.map((j) => `'${j.name}' (${j.pullRefusal})`).join('; ');
245+
clauses.push(
246+
`${one ? 'its enabled job' : `${jobsWithBadPull.length} of its enabled jobs`} ${list} `
247+
+ `${one ? 'has' : 'have'} a \`pull\` that does not bind, so this install door cannot run ${one ? 'it' : 'them'}: `
248+
+ 'the job would be installed and never scheduled. Declare the mapping the `pull` names in the package, with '
249+
+ 'a `connectorSource`, or correct the `pull` as the refusal says — `os validate` refuses the same `pull`.',
250+
);
251+
}
239252
if (what.hooks.length > 0) {
240253
const one = what.hooks.length === 1;
241254
clauses.push(
@@ -971,7 +984,11 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
971984
// - a hook with no `body`: its function-name `handler` can never
972985
// name the package's own code on this door, so it installed and
973986
// either never fired or bound by name to code the package does
974-
// not ship.
987+
// not ship;
988+
// - a job whose `pull` does not bind (a mapping the package does
989+
// not declare, one with no `connectorSource`): judged by the
990+
// binder's own `judgeJobPull` — it used to install and never be
991+
// scheduled.
975992
// The judgements are the runtime binder's own, so the door and the
976993
// binder cannot disagree about what this door can run.
977994
//
@@ -982,8 +999,9 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
982999
// a disabled one is never scheduled on any door; every hook is
9831000
// judged, since a hook has no on/off switch. ⛔ Rehydrate is not
9841001
// gated, for the id gate's reason: an entry an older build installed
985-
// still rehydrates — its unrunnable job is reported, not run, and its
986-
// hook with no `body` is warned and NOT bound
1002+
// still rehydrates — its unrunnable job is reported, not run (an
1003+
// unbindable `pull` is warned by the binder and NOT scheduled), and
1004+
// its hook with no `body` is warned and NOT bound
9871005
// ({@link bindArtifactHandlers}).
9881006
const unrunnable = await this.unrunnableCode(ctx, manifest, manifestId);
9891007
if (unrunnable.jobs.length > 0 || unrunnable.hooks.length > 0) {
@@ -1834,9 +1852,11 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
18341852
* [#21489, #21585] The code in `manifest` this door cannot run, which the
18351853
* install route refuses:
18361854
*
1837-
* - the enabled jobs with no `body`, or with a `body` the declaration
1838-
* refuses (`collectJobsWithoutBody`, which reads the jobs the
1839-
* binder schedules and judges a body by the parse the binder binds by);
1855+
* - the enabled jobs with no `body`, with a `body` the declaration
1856+
* refuses, or with a `pull` that does not bind
1857+
* (`collectJobsWithoutBody`, which reads the jobs the binder schedules,
1858+
* judges a body by the parse the binder binds by and a pull by the
1859+
* binder's own `judgeJobPull`);
18401860
* - the hooks with no `body` (`collectHooksWithoutBody`, the judgement the
18411861
* binder withholds by on this door's rehydrate).
18421862
*
@@ -1860,7 +1880,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
18601880
if (!collectJobs) {
18611881
ctx.logger?.warn?.(
18621882
`[MarketplaceInstallLocal] this runtime has no collectJobsWithoutBody — the jobs of ${manifestId} are not judged, `
1863-
+ 'so a job with no runnable `body` installs and is never run. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.',
1883+
+ 'so a job with no runnable `body` or `pull` installs and is never run. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.',
18641884
);
18651885
}
18661886
if (!collectHooks) {

0 commit comments

Comments
 (0)