diff --git a/.changeset/21585-hook-refusal-install-local.md b/.changeset/21585-hook-refusal-install-local.md new file mode 100644 index 00000000000..c26ac77acb6 --- /dev/null +++ b/.changeset/21585-hook-refusal-install-local.md @@ -0,0 +1,22 @@ +--- +'@objectstack/runtime': minor +'@objectstack/cloud-connection': minor +--- + +fix(runtime,cloud-connection)!: install-local refuses a hook with no `body` and a job `body` that does not bind, and withholds such a hook on rehydrate (#21585) + +Clause-②: yes (narrowing) + + + +**BREAKING**: `os package install` (the install-local door, `POST /api/v1/marketplace/install-local`) now refuses two more kinds of package it used to install with a 200: + +- **A hook with no `body`.** A hook in the deprecated function-name `handler` form names code that travels only in an artifact's runtime module, never in the package JSON this door installs. Such a hook used to install and then either never fire or bind by name to a function the package does not ship. Every hook is judged, since a hook has no on/off switch. A hook that carries both a `body` and a `handler` installs as before: its `body` wins. +- **An enabled job whose `body` does not bind.** The door used to judge only that a job `body` was present. It now judges that the body binds, by the declaration's own parse of `JobSchema.body`, the same parse the scheduler binds by. So a job whose `body` is an expression (L1) body, or carries `body.timeoutMs`, is refused instead of installed and never scheduled. + +- **The refusal.** The install answers `422` with `VALIDATION_ERROR`, the answer the door already gives an enabled job with no `body`. One answer names everything the door cannot run: each hook and the function its `handler` names, each job and its handler, and each refused job `body` with the key the declaration refuses. Nothing is installed: nothing is registered, persisted, bound or scheduled. `os package install` exits non-zero and prints the code beside the status. +- **Rehydrate.** A package installed by an earlier version keeps rehydrating after a restart. Its body hooks bind as before. A hook of it with no `body` is reported at `warn` by name and is **not bound**: this door carries no runtime module, so the hook's `handler` can never name the package's own code. Its job with no runnable `body` is reported and not run, as before. +- **Runtime.** The binder exports the two judgements the door reads: `collectHooksWithoutBody`, and `collectJobsWithoutBody`, which also names a job whose `body` does not bind. `bindAppArtifactHandlers` takes `withholdHooksWithoutBody`, which a door that carries no runtime module sets, and reports the hooks it withheld as `withheldHooks`. +- **Unchanged:** a boot that loads the artifact's runtime module (`os start --artifact`, a `defineStack` config) binds an app's handler hooks to its own functions exactly as before. Hooks authored through the metadata API are unchanged too. A package whose hooks carry a `body` and whose enabled jobs carry a valid `body` installs exactly as before. + +The route for a refused package: give each hook a `body` (sandboxed JS, the form actions and jobs use), and correct each job `body` to the declared shape. That shape is a sandboxed JS body whose time limit is the job's own `timeoutMs`, and `os validate` reports the same refusal. Alternatively, boot the artifact with `os start --artifact`, which loads its runtime module. This ships as `minor`, under the launch-window convention for narrowings of an accept set. diff --git a/content/docs/automation/jobs.mdx b/content/docs/automation/jobs.mdx index 245a2b1977a..1107a4a6180 100644 --- a/content/docs/automation/jobs.mdx +++ b/content/docs/automation/jobs.mdx @@ -170,8 +170,9 @@ export const CloseStaleTasksJob = defineJob({ job's `body` through the same binder. A `handler` is code: it travels only in the artifact's runtime module, so it runs only on a boot that loads that module (a config, or `os start --artifact`). `os package install` therefore refuses a - package whose enabled job has no `body`, with `422 VALIDATION_ERROR` and the - remedy: give the job a `body`, or boot it with `os start --artifact`. + package whose enabled job has no `body`, or a `body` that does not bind (an + expression body, or one carrying `body.timeoutMs`), with `422 VALIDATION_ERROR` + and the remedy: give the job a valid `body`, or boot it with `os start --artifact`. Uninstalling a package stops its scheduled jobs at once, and a reinstall whose new version drops a job stops that job. diff --git a/packages/cli/test/package-install-local-hooks.integration.test.ts b/packages/cli/test/package-install-local-hooks.integration.test.ts new file mode 100644 index 00000000000..c413e5b7d5f --- /dev/null +++ b/packages/cli/test/package-install-local-hooks.integration.test.ts @@ -0,0 +1,507 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21585 — install-local refuses the code it cannot run: a hook with no `body`, + * and a job whose `body` the declaration refuses. A package installed by an + * earlier build still rehydrates, its hook with no `body` warned and NOT bound. + * + * ## The defect, measured at this door before the fix + * + * A hook in the deprecated function-name `handler` form, with no `body`, names + * code that travels only in an artifact's runtime module — never in the package + * JSON `os package install` sends. Such a package installed with exit 0, and + * its hook either never fired or bound by name to a function the package does + * not ship, hot and after a restart. A job whose `body` + * the declaration refuses (an expression body, a `body.timeoutMs`) installed + * with exit 0 and was never scheduled. Only a server warn said either. + * + * ## What each `it` reads + * + * One host runtime, booted from an artifact whose runtime module ships a + * function and whose own hook names it in the `handler` form. Two boots of one + * home: + * + * 1. INSTALL — a package whose hook has no `body` (naming the host's function) + * is REFUSED, with its code and remedy, and nothing of it is installed; the + * two off-spec job bodies are refused the same way; the body-hook control + * installs and fires; a valid body job installs and runs; the host's own + * handler hook fires (the `--artifact` boot is unchanged); + * 2. RESTART — a ledger entry an earlier build wrote, carrying the same hook + * with no `body` beside a body hook, rehydrates: its object answers, its + * body hook fires, its hook with no `body` runs nothing and is warned by + * name. + * + * Every reading goes through a door a user uses: the CLI's own output and exit + * code for the install, the data route for what a hook or a job wrote. + * + * ## Spawn shape + * + * Shared with `package-install-local-jobs.integration.test.ts` (#21489): the + * tsx source entry, one process group per `os start`, every workspace package — + * `@objectstack/runtime` and `@objectstack/cloud-connection` included — + * resolved through its `exports` to `dist/`, so an ablation of either package's + * source reaches this file only after that package is rebuilt. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { spawn, type ChildProcess } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + CLI, + childEnv, + E2E_SECRET_KEY, + portContentionError, + portDriftError, + probeThroughChild, + randomPort, + TSX, +} from './helpers/serve-process.js'; + +/** The banner's tail — every row above it has printed. */ +const READY = /Press Ctrl\+C to stop/; +const BOOT_TIMEOUT_MS = 180_000; +/** How long a phase waits for a 1-second interval job to have written a row. */ +const RUN_WAIT_MS = 20_000; + +/** The development dev-admin seed — see the #21321 sibling for why it is the operator on every boot. */ +const EMAIL = 'admin@objectos.ai'; +const PASSWORD = 'admin123'; + +/** The host's function, shipped in its runtime module, and the host's own hook naming it. */ +const HOST_FN = 'host_stamp'; +const HOST_TASK = 'host_task'; +const HOST_TICK = 'host_tick'; + +/** A task object whose `status` a hook appends its stamp to, so every hook that ran is visible on the row. */ +function taskObject(name: string) { + return { + name, + label: 'Task', + sharingModel: 'public_read_write', + fields: { name: { type: 'text', label: 'Name' }, status: { type: 'text', label: 'Status' } }, + }; +} + +function tickObject(name: string) { + return { name, label: 'Tick', sharingModel: 'public_read_write', fields: { name: { type: 'text', label: 'Name' } } }; +} + +function bodyHook(name: string, object: string) { + return { + name, + object, + events: ['beforeInsert'], + onError: 'abort', + body: { language: 'js', source: "ctx.input.status = (typeof ctx.input.status === 'string' ? ctx.input.status : '') + 'body';" }, + }; +} + +/** The deprecated form: a function NAME, no `body`. */ +function handlerHook(name: string, object: string) { + return { name, object, events: ['beforeInsert'], onError: 'abort', handler: HOST_FN }; +} + +function bodyJob(name: string, object: string, body: Record = {}) { + return { + name, + schedule: { type: 'interval', intervalMs: 1000 }, + body: { + language: 'js', + capabilities: ['api.write'], + source: `await ctx.api.object('${object}').insert({ name: '${name}' });`, + ...body, + }, + timeoutMs: 10_000, + enabled: true, + }; +} + +/** + * The RUNTIME the packages are installed into: an `--artifact` app whose + * runtime module ships `HOST_FN`, and whose own hook names it in the `handler` + * form — the boot that carries its runtime module, which must keep binding it. + */ +const HOST_ARTIFACT = { + manifest: { id: 'com.example.host', namespace: 'host', version: '0.1.0', type: 'app', name: 'Host' }, + requires: ['job'], + objects: [taskObject(HOST_TASK), tickObject(HOST_TICK)], + hooks: [{ name: 'host_own_stamp', object: HOST_TASK, events: ['beforeInsert'], onError: 'abort', handler: HOST_FN }], + runtimeModule: './runtime.mjs', +}; +const HOST_RUNTIME_MODULE = + 'export const functions = {\n' + + ` ${HOST_FN}: async (ctx) => {\n` + + ' const d = ctx && ctx.input && ctx.input.data ? ctx.input.data : ctx.input;\n' + + " d.status = (typeof d.status === 'string' ? d.status : '') + 'host';\n" + + ' },\n' + + '};\n'; + +const HANDLER_APP_ID = 'com.example.handlerhooks'; +const HANDLER_TASK = 'handler_hooks_task'; +const HANDLER_HOOK = 'handler_hooks_by_name'; +/** The refused package: a hook with no `body`, beside a body hook. */ +const HANDLER_ARTIFACT = { + manifest: { id: HANDLER_APP_ID, namespace: 'handler_hooks', version: '0.1.0', type: 'app', name: 'Handler Hooks' }, + objects: [taskObject(HANDLER_TASK)], + hooks: [handlerHook(HANDLER_HOOK, HANDLER_TASK), bodyHook('handler_hooks_body', HANDLER_TASK)], +}; + +const BODY_APP_ID = 'com.example.bodyhooks'; +const BODY_TASK = 'body_hooks_task'; +/** The control: its one hook carries a `body`. */ +const BODY_ARTIFACT = { + manifest: { id: BODY_APP_ID, namespace: 'body_hooks', version: '0.1.0', type: 'app', name: 'Body Hooks' }, + objects: [taskObject(BODY_TASK)], + hooks: [bodyHook('body_hooks_body', BODY_TASK)], +}; + +const EXPR_APP_ID = 'com.example.exprjob'; +const EXPR_JOB = 'expr_job_tick'; +const EXPR_ARTIFACT = { + manifest: { id: EXPR_APP_ID, namespace: 'expr_job', version: '0.1.0', type: 'app', name: 'Expr Job' }, + objects: [tickObject('expr_job_tick')], + jobs: [{ name: EXPR_JOB, schedule: { type: 'interval', intervalMs: 1000 }, enabled: true, body: { language: 'expression', source: '1 + 1' } }], +}; + +const LIMIT_APP_ID = 'com.example.limitjob'; +const LIMIT_JOB = 'limit_job_tick'; +const LIMIT_ARTIFACT = { + manifest: { id: LIMIT_APP_ID, namespace: 'limit_job', version: '0.1.0', type: 'app', name: 'Limit Job' }, + objects: [tickObject('limit_job_tick')], + jobs: [bodyJob(LIMIT_JOB, HOST_TICK, { timeoutMs: 5_000 })], +}; + +const GOOD_APP_ID = 'com.example.goodjob'; +const GOOD_JOB = 'good_job_tick'; +const GOOD_ARTIFACT = { + manifest: { id: GOOD_APP_ID, namespace: 'good_job', version: '0.1.0', type: 'app', name: 'Good Job' }, + objects: [tickObject('good_job_tick')], + jobs: [bodyJob(GOOD_JOB, HOST_TICK)], +}; + +/** The entry an EARLIER build wrote for a package carrying a hook with no `body`, flattened as the install route persists it. */ +const LEGACY_APP_ID = 'com.example.legacyhooks'; +const LEGACY_TASK = 'legacy_hooks_task'; +const LEGACY_HOOK = 'legacy_hooks_by_name'; +const LEGACY_ENTRY = { + packageId: LEGACY_APP_ID, + versionId: 'local', + manifestId: LEGACY_APP_ID, + version: '0.1.0', + manifest: { + id: LEGACY_APP_ID, + namespace: 'legacy_hooks', + version: '0.1.0', + type: 'app', + name: 'Legacy Hooks', + objects: [taskObject(LEGACY_TASK)], + hooks: [handlerHook(LEGACY_HOOK, LEGACY_TASK), bodyHook('legacy_hooks_body', LEGACY_TASK)], + }, + installedAt: '2026-01-01T00:00:00.000Z', + installedBy: 'admin', + withSampleData: false, +}; + +const groups: ChildProcess[] = []; +const dirs: string[] = []; + +interface LiveStart { + child: ChildProcess; + base: string; + output: () => string; +} + +function bootStart(cwd: string, home: string, port: string, extra: string[] = []): Promise { + return new Promise((resolveBoot, rejectBoot) => { + const child = spawn(TSX, [CLI, 'start', '-p', port, '--home', home, '--auth-secret', E2E_SECRET_KEY, '--no-ui', ...extra], { + cwd, + // `childEnv`, never a bare `...process.env` — see its header. Package + // scheduled work is OFF by default in every posture (#17396); this + // runtime is one that runs it. + env: childEnv({ + NO_COLOR: '1', + OS_CLOUD_URL: 'off', + OS_LOG_LEVEL: 'warn', + OS_SECRET_KEY: E2E_SECRET_KEY, + OS_AUTOMATION_SCHEDULED_WORK_ENABLED: 'true', + }), + stdio: ['ignore', 'pipe', 'pipe'], + // Own process group: `os start` supervises a `serve` grandchild. + detached: true, + }); + groups.push(child); + let out = ''; + let settled = false; + const settle = (err: Error | null) => { + if (settled) return; + settled = true; + clearTimeout(timer); + if (err) rejectBoot(err); + else resolveBoot({ child, base: `http://localhost:${port}`, output: () => out }); + }; + const timer = setTimeout( + () => settle(new Error(`os start never printed ${READY}\n--- output ---\n${out.slice(-4000)}`)), + BOOT_TIMEOUT_MS, + ); + const onData = (d: unknown) => { + out += String(d); + // The child is the authority on the port it bound. + if (READY.test(out)) settle(portDriftError(out, 'os start', port)); + }; + child.stdout?.on('data', onData); + child.stderr?.on('data', onData); + child.on('exit', (code) => + settle(portContentionError(out, 'os start', port) + ?? new Error(`os start exited ${String(code)} before ${READY}\n--- output ---\n${out.slice(-4000)}`)), + ); + }); +} + +async function stopGroup(child: ChildProcess): Promise { + if (child.pid === undefined || child.exitCode !== null || child.signalCode !== null) return; + await new Promise((done) => { + const give = setTimeout(() => { + try { process.kill(-child.pid!, 'SIGKILL'); } catch { /* group already gone */ } + done(); + }, 15_000); + child.once('exit', () => { clearTimeout(give); done(); }); + try { process.kill(-child.pid!, 'SIGTERM'); } catch { clearTimeout(give); done(); } + }); +} + +interface Answer { status: number; body: any } + +/** One exchange against the running `os start`, attributed to the child if the transport fails. ⛔ No assertion inside it. */ +function http(live: LiveStart, method: string, path: string, token: string, body?: unknown): Promise { + return probeThroughChild( + { + child: live.child, + transcript: () => `\n--- child output ---\n${live.output().slice(-4000)}`, + label: 'package-install-local-hooks', + what: `${method} ${path}`, + }, + async () => { + const r = await fetch(`${live.base}${path}`, { + method, + headers: { + origin: live.base, + ...(body !== undefined ? { 'content-type': 'application/json' } : {}), + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + ...(body !== undefined ? { body: JSON.stringify(body) } : {}), + }); + const text = await r.text(); + let parsed: any = text; + try { parsed = JSON.parse(text); } catch { /* keep the text */ } + return { status: r.status, body: parsed }; + }, + ); +} + +async function authenticate(live: LiveStart): Promise { + const res = await http(live, 'POST', '/api/v1/auth/sign-in/email', '', { email: EMAIL, password: PASSWORD }); + const token = res.body?.token; + if (res.status !== 200 || typeof token !== 'string') { + throw new Error(`auth answered ${res.status}: ${JSON.stringify(res.body)}\n--- output ---\n${live.output().slice(-3000)}`); + } + return token; +} + +/** + * `os package install ./dist/objectstack.json` against the running runtime. + * ⛔ Asynchronous on purpose — see the #21321 sibling: a `spawnSync` stops this + * process draining the server's pipes for the whole install. + */ +function packageInstall(appDir: string, live: LiveStart): Promise<{ exit: number | null; output: string }> { + return new Promise((done) => { + const child = spawn(TSX, [CLI, 'package', 'install', './dist/objectstack.json', '--runtime', live.base, '--email', EMAIL, '--password', PASSWORD], { + cwd: appDir, + env: childEnv({ NO_COLOR: '1' }), + stdio: ['ignore', 'pipe', 'pipe'], + }); + let output = ''; + child.stdout?.on('data', (d) => { output += String(d); }); + child.stderr?.on('data', (d) => { output += String(d); }); + const timer = setTimeout(() => child.kill('SIGKILL'), 120_000); + child.on('close', (code) => { clearTimeout(timer); done({ exit: code, output }); }); + }); +} + +/** The record a `POST /api/v1/data/:object` answered with, whichever envelope it came in. */ +const record = (a: Answer) => a.body?.data ?? a.body?.record ?? a.body; + +/** The rows of a `GET /api/v1/data/:object` list answer, whichever envelope it came in. */ +function rowsOf(answer: Answer): any[] { + const b = answer.body?.data ?? answer.body; + if (Array.isArray(b)) return b; + if (Array.isArray(b?.records)) return b.records; + if (Array.isArray(b?.items)) return b.items; + return []; +} + +/** Wait (bounded) until `job` has written a row into `object` — a 1-second interval job is read for a while, not once. */ +async function awaitRuns(live: LiveStart, token: string, object: string, job: string): Promise { + const deadline = Date.now() + RUN_WAIT_MS; + const read = () => http(live, 'GET', `/api/v1/data/${object}?name=${encodeURIComponent(job)}&limit=500`, token); + let answer = await read(); + while (rowsOf(answer).length === 0 && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 500)); + answer = await read(); + } + return answer; +} + +interface InstallRun { exit: number | null; output: string } +const readings: { + handlerInstall?: InstallRun; + bodyInstall?: InstallRun; + exprInstall?: InstallRun; + limitInstall?: InstallRun; + goodInstall?: InstallRun; + installed?: Answer; + bodyInsert?: Answer; + hostInsert?: Answer; + goodRuns?: Answer; + limitRows?: Answer; + legacyInsert?: Answer; + output: Record; +} = { output: {} }; + +beforeAll(async () => { + const root = mkdtempSync(join(tmpdir(), 'install-local-hooks-')); + dirs.push(root); + const write = (dir: string, artifact: unknown) => { + mkdirSync(join(dir, 'dist'), { recursive: true }); + writeFileSync(join(dir, 'dist', 'objectstack.json'), JSON.stringify(artifact, null, 2), 'utf8'); + }; + const apps = { + handler: join(root, 'handler-app'), + body: join(root, 'body-app'), + expr: join(root, 'expr-app'), + limit: join(root, 'limit-app'), + good: join(root, 'good-app'), + }; + write(apps.handler, HANDLER_ARTIFACT); + write(apps.body, BODY_ARTIFACT); + write(apps.expr, EXPR_ARTIFACT); + write(apps.limit, LIMIT_ARTIFACT); + write(apps.good, GOOD_ARTIFACT); + + // The runtime boots the HOST artifact — never a package — so the packages + // reach it only through the install, or through the ledger. + const runtimeDir = join(root, 'runtime'); + mkdirSync(runtimeDir, { recursive: true }); + const hostArtifact = join(runtimeDir, 'host.json'); + writeFileSync(hostArtifact, JSON.stringify(HOST_ARTIFACT, null, 2), 'utf8'); + writeFileSync(join(runtimeDir, 'runtime.mjs'), HOST_RUNTIME_MODULE, 'utf8'); + const home = join(runtimeDir, 'home'); + const port = randomPort(); + + // ── boot 1: the host; hot installs ───────────────────────────────────── + const first = await bootStart(runtimeDir, home, port, ['--artifact', hostArtifact]); + const token = await authenticate(first); + readings.handlerInstall = await packageInstall(apps.handler, first); + readings.bodyInstall = await packageInstall(apps.body, first); + readings.exprInstall = await packageInstall(apps.expr, first); + readings.limitInstall = await packageInstall(apps.limit, first); + readings.goodInstall = await packageInstall(apps.good, first); + readings.installed = await http(first, 'GET', '/api/v1/marketplace/install-local', token); + readings.bodyInsert = await http(first, 'POST', `/api/v1/data/${BODY_TASK}`, token, { name: 'control' }); + readings.hostInsert = await http(first, 'POST', `/api/v1/data/${HOST_TASK}`, token, { name: 'host' }); + readings.goodRuns = await awaitRuns(first, token, HOST_TICK, GOOD_JOB); + readings.limitRows = await http(first, 'GET', `/api/v1/data/${HOST_TICK}?name=${LIMIT_JOB}&limit=500`, token); + readings.output.install = first.output(); + await stopGroup(first.child); + + // ── boot 2: same host, home and cwd, plus a ledger entry an earlier build wrote ── + const ledger = join(runtimeDir, '.objectstack', 'installed-packages'); + mkdirSync(ledger, { recursive: true }); + writeFileSync(join(ledger, `${LEGACY_APP_ID}.json`), JSON.stringify(LEGACY_ENTRY, null, 2), 'utf8'); + const second = await bootStart(runtimeDir, home, port, ['--artifact', hostArtifact]); + const token2 = await authenticate(second); + readings.legacyInsert = await http(second, 'POST', `/api/v1/data/${LEGACY_TASK}`, token2, { name: 'legacy' }); + readings.output.restart = second.output(); + await stopGroup(second.child); +}, 2 * BOOT_TIMEOUT_MS + 6 * 120_000 + RUN_WAIT_MS); + +afterAll(async () => { + for (const child of groups) await stopGroup(child); + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); +}, 60_000); + +const transcript = (phase: string) => `\n--- ${phase} output ---\n${(readings.output[phase] ?? '').slice(-3000)}`; + +describe('#21585: install-local refuses a hook with no body', () => { + it('a package whose hook has no body is REFUSED — non-zero exit, its code and remedy — and nothing of it is installed', () => { + const run = readings.handlerInstall!; + expect(run.exit, run.output).toBe(1); + // The CLI names the code the runtime answered with, and the remedy. + expect(run.output).toMatch(/Install failed \(422 VALIDATION_ERROR\)/); + expect(run.output).toContain(HANDLER_HOOK); + expect(run.output).toMatch(/give the hook a `body`/i); + expect(run.output).toMatch(/os start --artifact/); + const listing = readings.installed!; + expect(listing.status, JSON.stringify(listing.body)).toBe(200); + expect(JSON.stringify(listing.body), 'a refused package must leave nothing in the ledger').not.toContain(HANDLER_APP_ID); + }); + + it('control: the body-hook package installs (exit 0), and its hook fires on insert', () => { + const run = readings.bodyInstall!; + expect(run.exit, run.output).toBe(0); + expect(run.output).toMatch(/Package installed into the running kernel/); + const inserted = readings.bodyInsert!; + expect(inserted.status, JSON.stringify(inserted.body)).toBe(201); + expect(record(inserted)?.status, `null = the body hook never ran${transcript('install')}`).toBe('body'); + }); + + it("the `--artifact` boot is unchanged: the host's own handler hook binds to its own runtime module and fires", () => { + const inserted = readings.hostInsert!; + expect(inserted.status, JSON.stringify(inserted.body)).toBe(201); + expect(record(inserted)?.status, `the host's own handler hook did not fire${transcript('install')}`).toBe('host'); + }); +}); + +describe('#21585: install-local refuses a job whose body the declaration refuses', () => { + it('an expression (L1) job body is refused — non-zero exit, the refused key named', () => { + const run = readings.exprInstall!; + expect(run.exit, run.output).toBe(1); + expect(run.output).toMatch(/Install failed \(422 VALIDATION_ERROR\)/); + expect(run.output).toContain(EXPR_JOB); + expect(run.output).toContain('body.language'); + expect(JSON.stringify(readings.installed!.body)).not.toContain(EXPR_APP_ID); + }); + + it('a job body carrying timeoutMs is refused — non-zero exit, the one limit named — and it never runs', () => { + const run = readings.limitInstall!; + expect(run.exit, run.output).toBe(1); + expect(run.output).toMatch(/Install failed \(422 VALIDATION_ERROR\)/); + expect(run.output).toContain(LIMIT_JOB); + expect(run.output).toContain('body.timeoutMs'); + expect(JSON.stringify(readings.installed!.body)).not.toContain(LIMIT_APP_ID); + expect(rowsOf(readings.limitRows!)).toEqual([]); + }); + + it('control: a valid body job installs (exit 0) and runs on its schedule', () => { + const run = readings.goodInstall!; + expect(run.exit, run.output).toBe(0); + const answer = readings.goodRuns!; + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + expect(rowsOf(answer).length, `the installed body job never ran${transcript('install')}`).toBeGreaterThan(0); + }); +}); + +describe('#21585: rehydrate of an entry an earlier build installed', () => { + it('the package still rehydrates and its body hook fires; its hook with no body runs nothing', () => { + const inserted = readings.legacyInsert!; + expect(inserted.status, `the rehydrated package's object does not answer${transcript('restart')}`).toBe(201); + expect(record(inserted)?.status, `'body' alone = only the body hook ran${transcript('restart')}`).toBe('body'); + }); + + it('…and the hook with no body is warned by name, NOT bound', () => { + const out = readings.output.restart ?? ''; + const line = out.split('\n').find((l) => l.includes(LEGACY_HOOK) && l.includes('NOT bound')); + expect(line, `no warn names the withheld hook${transcript('restart')}`).toBeDefined(); + }); +}); diff --git a/packages/cloud-connection/src/marketplace-install-local-hooks.test.ts b/packages/cloud-connection/src/marketplace-install-local-hooks.test.ts new file mode 100644 index 00000000000..cab2fddb4b6 --- /dev/null +++ b/packages/cloud-connection/src/marketplace-install-local-hooks.test.ts @@ -0,0 +1,239 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21585 — install-local REFUSES a package carrying a hook with no `body`, and + * WITHHOLDS such a hook when it rehydrates an entry an older build installed. + * + * A hook in the deprecated function-name `handler` form names code that + * travels only in an artifact's runtime module, never in the package JSON this + * door installs. Before this, such a package installed with a 200 and its hook + * either never fired (the name resolved nowhere) or bound by name to a function + * the package does not ship — with only a server warn, if anything, to say so. + * + * What this file pins, against the door's answer and the engine's bindings: + * + * - install: a hook with no `body` answers `422 VALIDATION_ERROR` naming the + * hook, its handler and both remedies, and the runtime is left exactly as it + * was found — nothing registered, persisted or bound; + * - one answer names everything the door cannot run — hooks and jobs alike; + * - the body-hook control installs and is bound, and a hook carrying both a + * `body` and a `handler` installs (its body wins); + * - rehydrate: a ledger entry written by an earlier build binds its body hook + * and NOT its hook with no `body`, which is warned by name. + * + * The judgement and the binder are the REAL `@objectstack/runtime` exports + * (resolved through its `exports`, i.e. its built `dist/`, like the plugin's own + * lazy import); the engine is a recording double of what the binder touches. + * That the withheld hook can no longer reach another app's function is pinned on + * a real engine where the binder lives (`app-artifact-handlers.test.ts` in + * `@objectstack/runtime`) and at the public door + * (`packages/cli/test/package-install-local-hooks.integration.test.ts`). + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +// The first load of the runtime's dist paid at module top, never inside a +// clocked `it` (the clocked-window rule, `scripts/check-test-source-alias.mjs`): +// the plugin reaches the same module through a dynamic `import()`. +import '@objectstack/runtime'; +import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js'; +import { installerAuthService, withInstallerGrants } from './install-local-principal.fixtures.js'; +import { LocalManifestSource } from './local-manifest-source.js'; + +const APP_ID = 'com.example.hooksapp'; +const OWNER = `app:${APP_ID}`; +const OBJECT = 'hooks_app_task'; + +const BODY_HOOK = { + name: 'hooks_app_stamp_body', + object: OBJECT, + events: ['beforeInsert'], + body: { language: 'js', source: "ctx.input.status = 'stamped';" }, +}; +const HANDLER_HOOK = { name: 'hooks_app_stamp_handler', object: OBJECT, events: ['beforeInsert'], handler: 'stamp_fn' }; +const HANDLER_JOB = { name: 'hooks_app_tick_handler', schedule: { type: 'interval', intervalMs: 1000 }, handler: 'tick' }; + +/** The compiled-artifact shape `os build` writes and `os package install` sends. */ +function artifact(hooks: unknown[], extra: Record = {}) { + return { + manifest: { id: APP_ID, namespace: 'hooks_app', version: '0.1.0', type: 'app', name: 'Hooks App' }, + objects: [{ name: OBJECT, label: 'Task', fields: { name: { type: 'text', label: 'Name' } } }], + hooks, + ...extra, + }; +} + +/** The engine surface the binder writes to, as state: the hooks handed to `bindHooks`, by owner. */ +function recordingEngine() { + let hooks: Array<{ name: string; packageId?: string }> = []; + const unregisterHooksByPackage = (packageId: string): number => { + const before = hooks.length; + hooks = hooks.filter((h) => h.packageId !== packageId); + return before - hooks.length; + }; + const bindHooks = vi.fn((list: Array<{ name: string }> | undefined, opts?: { packageId?: string }) => { + if (!Array.isArray(list) || list.length === 0) return; + if (opts?.packageId) unregisterHooksByPackage(opts.packageId); + for (const h of list) hooks.push({ name: h.name, packageId: opts?.packageId }); + }); + return { + bindHooks, + hooksFor: (packageId: string) => hooks.filter((h) => h.packageId === packageId).map((h) => h.name), + engine: { + syncSchemas: async () => undefined, + registerAction: () => undefined, + removeActionsByPackage: () => undefined, + unregisterHooksByPackage, + bindHooks, + }, + }; +} + +type Handler = (c: any) => Promise; + +function makeRawApp() { + const routes = new Map(); + return { + routes, + get: (p: string, h: Handler) => routes.set(`GET ${p}`, h), + post: (p: string, h: Handler) => routes.set(`POST ${p}`, h), + delete: (p: string, h: Handler) => routes.set(`DELETE ${p}`, h), + }; +} + +function makeC(body: any) { + const json = vi.fn((payload: any, status?: number) => ({ payload, status: status ?? 200 })); + return { + req: { + url: 'http://localhost:3000/api/v1/marketplace/install-local', + raw: new Request('http://localhost:3000/x'), + json: async () => body, + param: () => undefined, + }, + json, + }; +} + +/** The package ids handed to `manifest.register` — not the plugin's own Setup nav bundle. */ +const registered = (register: ReturnType) => + register.mock.calls.map(([m]) => (m as { id?: string })?.id).filter((id) => id === APP_ID); + +let dir: string; +beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'mil-hooks-')); }); +afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); }); + +async function bootPlugin() { + const rec = recordingEngine(); + const register = vi.fn(); + const rawApp = makeRawApp(); + const kernelHooks = new Map(); + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const services: Record = { + manifest: { register }, + auth: installerAuthService(), + objectql: withInstallerGrants(rec.engine), + }; + const ctx = { + hook: (e: string, h: any) => kernelHooks.set(e, h), + getService: (name: string) => { + if (name === 'http-server') return { getRawApp: () => rawApp }; + const svc = services[name]; + if (svc === undefined) throw new Error(`no ${name}`); + return svc; + }, + logger, + }; + const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); + await plugin.start(ctx as any); + await kernelHooks.get('kernel:ready')?.(); + const install = async (bundle: unknown) => + rawApp.routes.get('POST /api/v1/marketplace/install-local')!(makeC({ manifest: bundle })); + const warned = () => logger.warn.mock.calls.map((c) => String(c[0])); + return { install, rec, register, warned }; +} + +describe('#21585: install-local refuses a hook with no body', () => { + it('answers 422 VALIDATION_ERROR naming the hook, its handler and both remedies — and changes nothing', async () => { + const { install, rec, register } = await bootPlugin(); + + const res = await install(artifact([BODY_HOOK, HANDLER_HOOK])); + + expect(res.status).toBe(422); + expect(res.payload.success).toBe(false); + expect(res.payload.error.code).toBe('VALIDATION_ERROR'); + const message: string = res.payload.error.message; + expect(message).toContain(`Package ${APP_ID} was not installed: its hook '${HANDLER_HOOK.name}' (handler 'stamp_fn') has no \`body\``); + expect(message).toMatch(/give the hook a `body`/i); + expect(message).toContain('os start --artifact'); + // The runtime is left exactly as it was found. + expect(registered(register), 'a refused package must not be registered').toEqual([]); + expect(new LocalManifestSource(dir).read(APP_ID).entry, 'nor persisted').toBeNull(); + expect(rec.bindHooks, 'nor any of its hooks bound — not even its body hook').not.toHaveBeenCalled(); + }); + + it('a hook with neither a body nor a handler is refused too, named as such', async () => { + const { install } = await bootPlugin(); + + const res = await install(artifact([{ name: 'hooks_app_neither', object: OBJECT, events: ['beforeInsert'] }])); + + expect(res.status).toBe(422); + expect(res.payload.error.message).toContain("its hook 'hooks_app_neither' (no handler) has no `body`"); + }); + + it('one answer names everything the door cannot run — every hook and every job — so the author fixes them in one pass', async () => { + const { install } = await bootPlugin(); + const second = { ...HANDLER_HOOK, name: 'hooks_app_second', handler: 'other_fn' }; + + const res = await install(artifact([HANDLER_HOOK, second], { jobs: [HANDLER_JOB] })); + + expect(res.status).toBe(422); + const message: string = res.payload.error.message; + expect(message).toContain(`its enabled job '${HANDLER_JOB.name}' (handler 'tick') has no \`body\``); + expect(message).toContain(`2 of its hooks '${HANDLER_HOOK.name}' (handler 'stamp_fn'), 'hooks_app_second' (handler 'other_fn') have no \`body\``); + }); + + it('control: a package whose hooks carry a body installs, and its hook is bound under app:', async () => { + const { install, rec, register } = await bootPlugin(); + + const res = await install(artifact([BODY_HOOK])); + + expect(res.status, JSON.stringify(res.payload)).toBe(200); + expect(registered(register)).toEqual([APP_ID]); + expect(rec.hooksFor(OWNER)).toEqual([BODY_HOOK.name]); + }); + + it('a hook carrying both a body and a handler installs — its body wins, as in the binder', async () => { + const { install, rec } = await bootPlugin(); + const both = { ...BODY_HOOK, name: 'hooks_app_both', handler: 'stamp_fn' }; + + const res = await install(artifact([both])); + + expect(res.status, JSON.stringify(res.payload)).toBe(200); + expect(rec.hooksFor(OWNER)).toEqual(['hooks_app_both']); + }); +}); + +describe('#21585: rehydrate withholds a hook with no body that an older build installed', () => { + it('the body hook is bound; the hook with no body is NOT bound, and is warned by name', async () => { + const { manifest: meta, ...sections } = artifact([BODY_HOOK, HANDLER_HOOK]); + new LocalManifestSource(dir).write({ + packageId: APP_ID, + versionId: 'local', + manifestId: APP_ID, + version: '0.1.0', + // What the install route persists: the compiled bundle, flattened. + manifest: { ...meta, ...sections }, + installedAt: '2026-01-01T00:00:00.000Z', + installedBy: 'admin', + withSampleData: false, + }); + + const { rec, register, warned } = await bootPlugin(); + + expect(registered(register), 'the older entry still rehydrates').toEqual([APP_ID]); + expect(rec.hooksFor(OWNER)).toEqual([BODY_HOOK.name]); + expect(warned().some((m) => m.includes(HANDLER_HOOK.name) && m.includes('NOT bound'))).toBe(true); + }); +}); diff --git a/packages/cloud-connection/src/marketplace-install-local-jobs.test.ts b/packages/cloud-connection/src/marketplace-install-local-jobs.test.ts index 23039decb2a..f883fbaac05 100644 --- a/packages/cloud-connection/src/marketplace-install-local-jobs.test.ts +++ b/packages/cloud-connection/src/marketplace-install-local-jobs.test.ts @@ -22,6 +22,8 @@ * - refusal: a handler-only enabled job answers `422 VALIDATION_ERROR` naming * the job, its handler and both remedies, and the runtime is left exactly as * it was found — nothing registered, persisted or scheduled; + * - #21585: so does an enabled job whose `body` the declaration refuses (an + * expression body, a `body.timeoutMs`), naming the refused key; * - a package without jobs, and one whose handler-only job is DISABLED, * install unchanged. * @@ -323,6 +325,44 @@ describe('#21489: install-local refuses an enabled job with no body', () => { expect(jobs.svc.schedule).not.toHaveBeenCalled(); }); + it('#21585: an enabled job whose body is an expression (L1) is refused, naming the key the declaration refuses — and changes nothing', async () => { + const { install, jobs, register } = await bootPlugin(); + const l1 = { name: 'jobs_app_expr', schedule: INTERVAL, body: { language: 'expression', source: '1 + 1' } }; + + const res = await install(artifact([BODY_JOB, l1])); + + expect(res.status).toBe(422); + expect(res.payload.error.code).toBe('VALIDATION_ERROR'); + const message: string = res.payload.error.message; + expect(message).toContain(`its enabled job 'jobs_app_expr' (body.language: `); + expect(message).toContain('has a `body` the declaration refuses'); + expect(message).toContain('os validate'); + expect(registered(register)).toEqual([]); + expect(new LocalManifestSource(dir).read(APP_ID).entry).toBeNull(); + expect(jobs.svc.schedule).not.toHaveBeenCalled(); + }); + + it('#21585: an enabled job whose body carries timeoutMs is refused, naming the one limit', async () => { + const { install, jobs } = await bootPlugin(); + const twoLimits = { ...BODY_JOB, name: 'jobs_app_two_limits', body: { ...BODY_JOB.body, timeoutMs: 5_000 } }; + + const res = await install(artifact([twoLimits])); + + expect(res.status).toBe(422); + expect(res.payload.error.code).toBe('VALIDATION_ERROR'); + expect(res.payload.error.message).toContain(`its enabled job 'jobs_app_two_limits' (body.timeoutMs: `); + expect(res.payload.error.message).toContain("job's own `timeoutMs`"); + expect(jobs.svc.schedule).not.toHaveBeenCalled(); + }); + + it('#21585: a DISABLED job with an off-spec body does not block the install', async () => { + const { install } = await bootPlugin(); + + const res = await install(artifact([{ name: 'jobs_app_off', schedule: INTERVAL, enabled: false, body: { language: 'expression', source: '1' } }])); + + expect(res.status, JSON.stringify(res.payload)).toBe(200); + }); + it('a package without jobs installs unchanged — the same answer, nothing scheduled', async () => { const { install, jobs, register } = await bootPlugin(); diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index 410bc0b9387..30d7300b1e5 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -27,10 +27,13 @@ * before anything else looks at it; an id the declaration refuses * answers `PLUGIN_MANIFEST_INVALID` (400 for an inline manifest, * 502 for a cloud snapshot) and nothing is registered or written. - * A package declaring an enabled job with no `body` answers - * `VALIDATION_ERROR` (422) the same way: a job's function-name - * `handler` is code no JSON door carries. The package's job bodies - * are scheduled on install and on every rehydrate. + * A package declaring code this door cannot run answers + * `VALIDATION_ERROR` (422) the same way: an enabled job with no + * `body`, or one whose `body` the declaration refuses (#21585), and + * a hook with no `body` (#21585) — a function-name `handler` is code + * no JSON door carries. The package's job bodies are scheduled on + * install and on every rehydrate; on a rehydrate, a hook with no + * `body` that an older build installed is warned and NOT bound. * * GET /api/v1/marketplace/install-local * → lists currently installed marketplace packages. Requires an @@ -162,44 +165,89 @@ const REGISTRY_WITHDRAWAL = 'registry.uninstallPackage'; const INSTALL_LOCAL_CAPABILITY = 'manage_metadata'; /** - * [#21489] The refusal of a package that declares an enabled job no JSON door - * can run: `VALIDATION_ERROR` / 422. + * [#21489, #21585] The refusal of a package that declares code this door cannot + * run — an enabled job with no `body` or with a `body` the declaration refuses, + * a hook with no `body`: `VALIDATION_ERROR` / 422. * * The code is the standard catalog's input-validation member. The condition is * that the install payload fails this door's acceptance rule — every enabled - * job carries a `body` — and the ledger's admission rule sends a generic - * validation condition to the standard member rather than to a registered - * synonym (`error-code-ledger.zod.ts`, "Registering a new code"). What the - * author does instead is the prescription the message carries. - * `PLUGIN_MANIFEST_INVALID` is deliberately not it: the manifest is valid — - * `os validate` passes it and `os start --artifact` runs it. + * job carries a `body` that binds, and every hook carries a `body` — and the + * ledger's admission rule sends a generic validation condition to the standard + * member rather than to a registered synonym (`error-code-ledger.zod.ts`, + * "Registering a new code"). What the author does instead is the prescription + * the message carries. `PLUGIN_MANIFEST_INVALID` is deliberately not it: that + * code answers the manifest's identity at this door, and a handler-form job or + * hook is a valid manifest — `os validate` passes it and `os start --artifact` + * runs it. One acceptance rule answers one code, so the off-spec job `body` + * (which `os validate` does refuse) is answered by the same refusal as the rest + * of the rule rather than splitting it. * * The status is 422, not the 400 / 502 split this door uses for an invalid - * manifest: the package is well-formed and this door cannot process it, which + * manifest id: the package is well-formed JSON this door cannot process, which * holds whichever branch supplied it — a catalog package declaring a handler job * is no upstream fault. 422 derives `VALIDATION_ERROR` * (`standardErrorCodeForHttpStatus`), so code and status agree. */ -const JOB_WITHOUT_BODY_REFUSAL_CODE = 'VALIDATION_ERROR'; -const JOB_WITHOUT_BODY_REFUSAL_STATUS = 422; +const UNRUNNABLE_REFUSAL_CODE = 'VALIDATION_ERROR'; +const UNRUNNABLE_REFUSAL_STATUS = 422; + +/** What this door cannot run in a package, as the runtime binder judges it. */ +interface UnrunnableCode { + jobs: ReadonlyArray<{ name: string; handler?: string; bodyRefusal?: string }>; + hooks: ReadonlyArray<{ name: string; handler?: string }>; +} + +/** `'name' (handler 'fn')` / `'name' (no handler)`, comma-joined. */ +function describeNamedHandlers(items: ReadonlyArray<{ name: string; handler?: string }>): string { + return items.map((i) => `'${i.name}' (${i.handler !== undefined ? `handler '${i.handler}'` : 'no handler'})`).join(', '); +} + +const capitalize = (text: string) => text.charAt(0).toUpperCase() + text.slice(1); /** - * The refusal sentence: which jobs, why this door cannot run them, and the two - * remedies — a `body` (every door), or an `--artifact` boot (which loads the - * runtime module a `handler` lives in). Names each job and the function its - * `handler` declares, so the author fixes them all in one pass. + * The refusal sentence: everything the door cannot run, why, and the remedies, + * one clause per kind — a job with no `body`, a job whose `body` the + * declaration refuses, a hook with no `body` — in one answer, so the author + * fixes them all in one pass. Each names the item and the function its + * `handler` declares (or the declaration's refusal of its `body`). */ -function describeJobsWithoutBody(manifestId: string, jobs: ReadonlyArray<{ name: string; handler?: string }>): string { - const list = jobs - .map((j) => `'${j.name}' (${j.handler !== undefined ? `handler '${j.handler}'` : 'no handler'})`) - .join(', '); - const one = jobs.length === 1; - return `Package ${manifestId} was not installed: ${one ? 'its enabled job' : `${jobs.length} of its enabled jobs`} ` - + `${list} ${one ? 'has' : 'have'} no \`body\`, so this install door cannot run ${one ? 'it' : 'them'}. ` - + "A job's `handler` names a `defineStack({ functions })` entry, which is code: it travels in the artifact's " - + 'runtime module, never in the package JSON this door installs, so the job would be installed and never run. ' - + 'Give the job a `body` (sandboxed JS, the form hooks and actions use, which travels with the package and runs ' - + 'on every door), or boot the artifact with `os start --artifact`, which loads its runtime module.'; +function describeUnrunnable(manifestId: string, what: UnrunnableCode): string { + const clauses: string[] = []; + const jobsWithoutBody = what.jobs.filter((j) => j.bodyRefusal === undefined); + const jobsWithBadBody = what.jobs.filter((j) => j.bodyRefusal !== undefined); + if (jobsWithoutBody.length > 0) { + const one = jobsWithoutBody.length === 1; + clauses.push( + `${one ? 'its enabled job' : `${jobsWithoutBody.length} of its enabled jobs`} ` + + `${describeNamedHandlers(jobsWithoutBody)} ${one ? 'has' : 'have'} no \`body\`, so this install door cannot run ${one ? 'it' : 'them'}. ` + + "A job's `handler` names a `defineStack({ functions })` entry, which is code: it travels in the artifact's " + + 'runtime module, never in the package JSON this door installs, so the job would be installed and never run. ' + + 'Give the job a `body` (sandboxed JS, the form hooks and actions use, which travels with the package and runs ' + + 'on every door), or boot the artifact with `os start --artifact`, which loads its runtime module.', + ); + } + if (jobsWithBadBody.length > 0) { + const one = jobsWithBadBody.length === 1; + const list = jobsWithBadBody.map((j) => `'${j.name}' (${j.bodyRefusal})`).join('; '); + clauses.push( + `${one ? 'its enabled job' : `${jobsWithBadBody.length} of its enabled jobs`} ${list} ` + + `${one ? 'has' : 'have'} a \`body\` the declaration refuses, so this install door cannot run ${one ? 'it' : 'them'}: ` + + 'the job would be installed and never scheduled. Correct the `body` to the declared shape (a sandboxed JS ' + + "body; the job's time limit is the job's own `timeoutMs`) — `os validate` reports the same refusal.", + ); + } + if (what.hooks.length > 0) { + const one = what.hooks.length === 1; + clauses.push( + `${one ? 'its hook' : `${what.hooks.length} of its hooks`} ${describeNamedHandlers(what.hooks)} ` + + `${one ? 'has' : 'have'} no \`body\`, so this install door cannot run ${one ? 'it' : 'them'}. ` + + "A hook's `handler` names a function, which is code: it travels in the artifact's runtime module, never in " + + "the package JSON this door installs, so this door can never bind it to the package's own code. " + + 'Give the hook a `body` (sandboxed JS, the form actions and jobs use, which travels with the package and ' + + 'runs on every door), or boot the artifact with `os start --artifact`, which loads its runtime module.', + ); + } + return `Package ${manifestId} was not installed: ${clauses.map((c, i) => (i === 0 ? c : capitalize(c))).join(' ')}`; } /** @@ -908,7 +956,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin { const manifestId = declaredId.data; if (inlineManifest) packageId = manifestId; - // 1c. [#21489] ⭐ A JOB THIS DOOR CANNOT RUN IS REFUSED, not installed. + // 1c. [#21489] ⭐ CODE THIS DOOR CANNOT RUN IS REFUSED, not installed. // A job runs on a JSON door only through its sandboxed `body`; its // deprecated `handler` names a `defineStack({ functions })` entry, // which is code and travels only in the artifact's runtime module — @@ -916,22 +964,36 @@ export class MarketplaceInstallLocalPlugin implements Plugin { // package installed with a 200 and its job was declared and never // scheduled, hot or after a restart, with nothing anywhere saying so. // + // [#21585] The same rule, completed: + // - a job whose `body` the declaration refuses (an expression body, + // a `body.timeoutMs`) is judged by whether it BINDS, not by + // whether it is present — it used to install and never run; + // - a hook with no `body`: its function-name `handler` can never + // name the package's own code on this door, so it installed and + // either never fired or bound by name to code the package does + // not ship. + // The judgements are the runtime binder's own, so the door and the + // binder cannot disagree about what this door can run. + // // Answered here, beside the id gate and ahead of the collision check, // the posture gate, the hot-register and the ledger write, so a // refused install leaves the runtime exactly as it found it and the // author can add the body and retry. Only an ENABLED job is judged: - // a disabled one is never scheduled on any door. ⛔ Rehydrate is not + // a disabled one is never scheduled on any door; every hook is + // judged, since a hook has no on/off switch. ⛔ Rehydrate is not // gated, for the id gate's reason: an entry an older build installed - // still rehydrates (its handler-only job is reported, not run). - const withoutBody = await this.jobsWithoutBody(ctx, manifest, manifestId); - if (withoutBody.length > 0) { + // still rehydrates — its unrunnable job is reported, not run, and its + // hook with no `body` is warned and NOT bound + // ({@link bindArtifactHandlers}). + const unrunnable = await this.unrunnableCode(ctx, manifest, manifestId); + if (unrunnable.jobs.length > 0 || unrunnable.hooks.length > 0) { return c.json({ success: false, error: { - code: JOB_WITHOUT_BODY_REFUSAL_CODE, - message: describeJobsWithoutBody(manifestId, withoutBody), + code: UNRUNNABLE_REFUSAL_CODE, + message: describeUnrunnable(manifestId, unrunnable), }, - }, JOB_WITHOUT_BODY_REFUSAL_STATUS); + }, UNRUNNABLE_REFUSAL_STATUS); } // 2. Conflict check — refuse to overwrite user-authored apps @@ -1696,9 +1758,16 @@ export class MarketplaceInstallLocalPlugin implements Plugin { * binder's job half and the call `AppPlugin` makes on `kernel:ready`: a job * `body` runs sandboxed on its schedule, hot and after a restart. A * handler-only job never reaches here on an install — the install route - * refuses it ({@link jobsWithoutBody}) — and on a rehydrate of an entry an + * refuses it ({@link unrunnableCode}) — and on a rehydrate of an entry an * older build installed it is reported at `warn` and not run. * + * [#21585] This door carries no runtime module, and says so to the binder + * (`withholdHooksWithoutBody`): a hook with no `body` is warned and NOT + * bound, because its function-name `handler` can never name the package's + * own code here. The install route refuses such a hook, so this fires only + * on the rehydrate of an entry an older build installed — which before this + * bound the hook by name to whatever function the engine held under it. + * * Before this, `manifest.register` was the whole install: the package's * actions and hooks were DECLARED and never bound, so every door refused * its script actions ("No handler registered" over MCP, 404 over REST) @@ -1736,7 +1805,20 @@ export class MarketplaceInstallLocalPlugin implements Plugin { + 'every door refuses those actions and the hooks never fire. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.', ); } else { - bind(ql, manifest, { appId: manifestId, logger: ctx.logger, source: 'MarketplaceInstallLocal' }); + const bound = bind(ql, manifest, { + appId: manifestId, + logger: ctx.logger, + source: 'MarketplaceInstallLocal', + withholdHooksWithoutBody: true, + }); + // A runtime that predates the option binds a hook with no `body` by + // name, as before — say so rather than answer as if it withheld it. + if (!Array.isArray((bound as { withheldHooks?: unknown } | undefined)?.withheldHooks)) { + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] this runtime's binder predates withholdHooksWithoutBody — a hook of ${manifestId} with no \`body\` ` + + 'is bound by its handler name, not withheld. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.', + ); + } } if (!schedule) { ctx.logger?.warn?.( @@ -1749,33 +1831,45 @@ export class MarketplaceInstallLocalPlugin implements Plugin { }; /** - * [#21489] The enabled jobs of `manifest` that carry no `body` — the jobs - * no JSON door can run, which the install route refuses. The judgement is - * the runtime binder's own (`collectJobsWithoutBody`, which reads the jobs - * the binder schedules), so the door and the binder cannot disagree about - * what this door can run. - * - * A runtime that predates the judgement judges nothing and says so: the - * install proceeds as it did before this gate existed. + * [#21489, #21585] The code in `manifest` this door cannot run, which the + * install route refuses: + * + * - the enabled jobs with no `body`, or with a `body` the declaration + * refuses (`collectJobsWithoutBody`, which reads the jobs the + * binder schedules and judges a body by the parse the binder binds by); + * - the hooks with no `body` (`collectHooksWithoutBody`, the judgement the + * binder withholds by on this door's rehydrate). + * + * Both judgements are the runtime binder's own, so the door and the binder + * cannot disagree about what this door can run. A runtime that predates a + * judgement judges nothing for it and says so: the install proceeds as it + * did before that gate existed. */ - private jobsWithoutBody = async ( + private unrunnableCode = async ( ctx: PluginContext, manifest: unknown, manifestId: string, - ): Promise> => { - let collect: typeof import('@objectstack/runtime')['collectJobsWithoutBody'] | undefined; + ): Promise => { + let collectJobs: typeof import('@objectstack/runtime')['collectJobsWithoutBody'] | undefined; + let collectHooks: typeof import('@objectstack/runtime')['collectHooksWithoutBody'] | undefined; try { const mod: any = await import('@objectstack/runtime'); - if (typeof mod?.collectJobsWithoutBody === 'function') collect = mod.collectJobsWithoutBody; + if (typeof mod?.collectJobsWithoutBody === 'function') collectJobs = mod.collectJobsWithoutBody; + if (typeof mod?.collectHooksWithoutBody === 'function') collectHooks = mod.collectHooksWithoutBody; } catch { /* reported below */ } - if (!collect) { + if (!collectJobs) { ctx.logger?.warn?.( `[MarketplaceInstallLocal] this runtime has no collectJobsWithoutBody — the jobs of ${manifestId} are not judged, ` - + 'so a job with no `body` installs and is never run. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.', + + 'so a job with no runnable `body` installs and is never run. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.', + ); + } + if (!collectHooks) { + ctx.logger?.warn?.( + `[MarketplaceInstallLocal] this runtime has no collectHooksWithoutBody — the hooks of ${manifestId} are not judged, ` + + 'so a hook with no `body` installs. Upgrade @objectstack/runtime alongside @objectstack/cloud-connection.', ); - return []; } - return collect(manifest); + return { jobs: collectJobs ? collectJobs(manifest) : [], hooks: collectHooks ? collectHooks(manifest) : [] }; }; /** diff --git a/packages/runtime/src/app-artifact-handlers.jobs.test.ts b/packages/runtime/src/app-artifact-handlers.jobs.test.ts index 4a8988e4005..2cfc44c806f 100644 --- a/packages/runtime/src/app-artifact-handlers.jobs.test.ts +++ b/packages/runtime/src/app-artifact-handlers.jobs.test.ts @@ -2,8 +2,9 @@ /** * #21489 — the binder's job half: `scheduleAppArtifactJobs`, the ONE place a - * declared job becomes a scheduled one, and `collectJobsWithoutBody`, the - * judgement the install-local door refuses on. + * declared job becomes a scheduled one, and `collectJobsWithoutBody`, + * the judgement the install-local door refuses on (#21585: no `body`, or a + * `body` the declaration refuses — the same judgement the binder binds by). * * Its callers are pinned where they live (`AppPlugin` below and in * `app-plugin.jobs.test.ts`; the install-local plugin in @@ -411,6 +412,40 @@ describe('#21489: collectJobsWithoutBody — what no JSON door can run', () => { }); }); +describe('#21585: a job body the declaration refuses is judged as unrunnable — by the judgement the binder binds by', () => { + const L1 = { name: 'l1_job', schedule: INTERVAL, body: { language: 'expression', source: '1 + 1' } }; + const TWO_LIMITS = { name: 'two_limits', schedule: INTERVAL, body: { ...WRITE_BODY, timeoutMs: 100 } }; + const GOOD = { name: 'good_job', schedule: INTERVAL, body: WRITE_BODY }; + + it('names an L1 expression body and a body carrying timeoutMs, each with the declaration\'s refusal', () => { + const named = collectJobsWithoutBody(pkg([L1, TWO_LIMITS, GOOD, { ...L1, name: 'l1_disabled', enabled: false }])); + + expect(named.map((j) => j.name)).toEqual(['l1_job', 'two_limits']); + // The key the refusal names, so the author knows where to look. + expect(named[0].bodyRefusal).toMatch(/^body\.language: /); + expect(named[1].bodyRefusal).toMatch(/^body\.timeoutMs: /); + // The sentence is the spec's, not a paraphrase: it names the one limit. + expect(named[1].bodyRefusal).toContain("job's own `timeoutMs`"); + }); + + it('a body with a handler beside it is judged by its body — the body wins, as it does in the binder', () => { + const named = collectJobsWithoutBody(pkg([{ ...L1, handler: 'tick' }, { ...GOOD, name: 'good_both', handler: 'tick' }])); + + expect(named).toEqual([{ name: 'l1_job', handler: 'tick', bodyRefusal: expect.stringMatching(/^body\.language: /) }]); + }); + + it('the door and the binder agree: every job named is NOT scheduled, every enabled body job not named IS', async () => { + const h = harness(); + const jobs = [L1, TWO_LIMITS, GOOD, { name: 'handler_only', schedule: INTERVAL, handler: 'tick' }]; + + const named = new Set(collectJobsWithoutBody(pkg(jobs)).map((j) => j.name)); + await h.schedule(pkg(jobs)); + + expect([...named].sort()).toEqual(['handler_only', 'l1_job', 'two_limits']); + expect([...h.jobs.scheduled.keys()]).toEqual(['good_job']); + }); +}); + describe('#21489: AppPlugin (the boot door) schedules a body job through the binder', () => { it('a body-only job — skipped at warn before — is scheduled on kernel:ready, and runs its body', async () => { const engine = recordingEngine(); diff --git a/packages/runtime/src/app-artifact-handlers.test.ts b/packages/runtime/src/app-artifact-handlers.test.ts index ec1b5959aad..5002ceca68f 100644 --- a/packages/runtime/src/app-artifact-handlers.test.ts +++ b/packages/runtime/src/app-artifact-handlers.test.ts @@ -22,7 +22,7 @@ import { describe, it, expect } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; -import { bindAppArtifactHandlers, appArtifactHandlerOwner } from './app-artifact-handlers.js'; +import { bindAppArtifactHandlers, appArtifactHandlerOwner, collectHooksWithoutBody } from './app-artifact-handlers.js'; const APP_ID = 'com.example.tasksapp'; const OWNER = appArtifactHandlerOwner(APP_ID); @@ -114,3 +114,85 @@ describe('#21321: bindAppArtifactHandlers', () => { expect(owned(ql, 'metadata-service')).toEqual(['tasks_app_task:authored_task']); }); }); + +/** + * #21585 — a hook with no `body` on a door that carries no runtime module. + * + * The engine resolves a hook's function-name `handler` against the bundle's + * `functions`, then against every function already registered on the engine, + * by bare name. A door that brings no runtime module (install-local) brings no + * function of the package's own, so such a hook could bind only to code the + * package does not ship. That door passes `withholdHooksWithoutBody`; a boot + * does not, and its own handler hooks bind as they always did. + */ +describe('#21585: hooks with no body — the judgement, and the door that withholds them', () => { + const OTHER_APP = 'com.example.otherapp'; + const STAMP = 'shared_stamp'; + /** Another app's function, registered on the same engine under its own owner. */ + const otherApp = { + manifest: { id: OTHER_APP, version: '0.1.0', type: 'app' }, + objects: [], + functions: { [STAMP]: (ctx: any) => { ctx.input.status = (ctx.input.status ?? '') + 'other'; } }, + }; + const HANDLER_ONLY = { name: 'tasks_app_handler_only', object: 'tasks_app_task', events: ['beforeInsert'], handler: STAMP }; + const BOTH = { ...HOOK, name: 'tasks_app_both', handler: STAMP }; + const pkg = (hooks: unknown[], extra: Record = {}) => ({ + manifest: { id: APP_ID, version: '0.1.0', type: 'app' }, + objects: [{ name: 'tasks_app_task', fields: {} }], + hooks, + ...extra, + }); + const recordingLogger = () => { + const warned: string[] = []; + return { warned, logger: { ...quiet, warn: (m: string) => { warned.push(String(m)); } } }; + }; + + it('collectHooksWithoutBody names every hook whose code is only a handler, or nothing — never one with a body', () => { + const NEITHER = { name: 'tasks_app_neither', object: 'tasks_app_task', events: ['beforeInsert'] }; + const STRING_BODY = { ...HANDLER_ONLY, name: 'tasks_app_string_body', body: 'not a body object' }; + + expect(collectHooksWithoutBody(pkg([HOOK, HANDLER_ONLY, BOTH, NEITHER, STRING_BODY]))).toEqual([ + { name: HANDLER_ONLY.name, handler: STAMP }, + { name: NEITHER.name }, + // A non-object `body` is not read as one by the engine: its handler would be resolved. + { name: STRING_BODY.name, handler: STAMP }, + ]); + expect(collectHooksWithoutBody(pkg([]))).toEqual([]); + expect(collectHooksWithoutBody({ manifest: { id: APP_ID } })).toEqual([]); + }); + + it('with withholdHooksWithoutBody, a hook with no body is NOT bound and is warned — no other app’s function runs for it', async () => { + const ql = new ObjectQL({ logger: quiet } as any); + bindAppArtifactHandlers(ql as any, otherApp, { appId: OTHER_APP, logger: quiet }); + const { warned, logger } = recordingLogger(); + + const bound = bindAppArtifactHandlers(ql as any, pkg([HANDLER_ONLY, HOOK]), { + appId: APP_ID, logger, withholdHooksWithoutBody: true, + }); + + expect(bound.withheldHooks).toEqual([HANDLER_ONLY.name]); + expect(bound.hooks, 'the body hook beside it still binds').toBe(1); + expect(await insertStatus(ql), 'the body hook fires; the withheld hook runs nothing').toBe('stamped'); + expect(warned.some((m) => m.includes(HANDLER_ONLY.name) && m.includes('NOT bound') && m.includes('`body`'))).toBe(true); + }); + + it('…and a hook carrying both a body and a handler binds its body under that option', async () => { + const ql = new ObjectQL({ logger: quiet } as any); + bindAppArtifactHandlers(ql as any, otherApp, { appId: OTHER_APP, logger: quiet }); + + const bound = bindAppArtifactHandlers(ql as any, pkg([BOTH]), { appId: APP_ID, logger: quiet, withholdHooksWithoutBody: true }); + + expect(bound.withheldHooks).toEqual([]); + expect(await insertStatus(ql)).toBe('stamped'); + }); + + it('a boot (no option) still binds an app’s own handler hook to its own function — unchanged', async () => { + const ql = new ObjectQL({ logger: quiet } as any); + const own = (ctx: any) => { ctx.input.status = (ctx.input.status ?? '') + 'own'; }; + + const bound = bindAppArtifactHandlers(ql as any, pkg([HANDLER_ONLY], { functions: { [STAMP]: own } }), { appId: APP_ID, logger: quiet }); + + expect(bound).toMatchObject({ hooks: 1, functions: 1, withheldHooks: [] }); + expect(await insertStatus(ql)).toBe('own'); + }); +}); diff --git a/packages/runtime/src/app-artifact-handlers.ts b/packages/runtime/src/app-artifact-handlers.ts index 903c0ea6c51..8ee17aa5a3f 100644 --- a/packages/runtime/src/app-artifact-handlers.ts +++ b/packages/runtime/src/app-artifact-handlers.ts @@ -65,11 +65,31 @@ * (the job service and the engine have registered), while install-local's * doors are already past that point. One implementation, two moments. * - * A job runs on a JSON door only through its `body`. Its deprecated `handler` - * names a `defineStack({ functions })` entry, which is code: it travels in the - * artifact's runtime module, which only `os start --artifact` loads, so no JSON - * door can ever resolve it. {@link collectJobsWithoutBody} names those jobs, and - * the install-local install route refuses a package that declares one enabled. + * A job runs on a JSON door only through a `body` that binds. Its deprecated + * `handler` names a `defineStack({ functions })` entry, which is code: it + * travels in the artifact's runtime module, which only `os start --artifact` + * loads, so no JSON door can ever resolve it. {@link collectJobsWithoutBody} + * names those jobs — and the ones whose `body` the declaration refuses + * (`judgeJobBody`) — and the install-local install route refuses a package that + * declares one enabled. + * + * ## A hook with no `body`, on a door that carries no runtime module (#21585) + * + * The same holds for a hook in the deprecated function-name `handler` form, with + * one difference that makes it worse than a job: the engine resolves a hook's + * `handler` against the bundle's `functions` AND, failing that, against every + * function already registered on the engine, by bare name (`HookSchema.handler` + * declares that fallback). A door that carries no runtime module brings no + * function of the package's own, so such a hook can never bind to the package's + * code — only to a function some other app registered, or to nothing. + * + * So a door that carries no runtime module — install-local — says so with + * {@link AppArtifactHandlerBindingOptions.withholdHooksWithoutBody}: every hook + * {@link collectHooksWithoutBody} names is withheld from `bindHooks`, warned and + * NOT bound. Its install route refuses a package that declares one, so this + * fires only on the rehydrate of an entry an older build installed. A boot + * (`AppPlugin`, `os start --artifact`, a `defineStack` config) carries its + * runtime module and passes no such option: its handler hooks bind unchanged. */ import type { PluginContext } from '@objectstack/core'; @@ -77,7 +97,7 @@ import type { IJobService, IObjectQLEngine, JobHandler, Logger } from '@objectst import { resolveScheduledWorkEnabled, SCHEDULED_WORK_DISABLED_REASON } from '@objectstack/types'; import { SEMCONV } from '@objectstack/observability'; import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js'; -import { hookBodyRunnerFactory, actionBodyRunnerFactory, jobBodyRunnerFactory } from './sandbox/body-runner.js'; +import { hookBodyRunnerFactory, actionBodyRunnerFactory, jobBodyRunnerFactory, judgeJobBody } from './sandbox/body-runner.js'; import { GLOBAL_ACTION_OBJECT_KEY } from './action-execution.js'; import { collectBundleActions, @@ -107,6 +127,14 @@ export interface AppArtifactHandlerBindingOptions { logger: Logger; /** Who is binding, for the log lines — `'AppPlugin'`, `'MarketplaceInstallLocal'`. */ source?: string; + /** + * [#21585] Set by a door that carries NO runtime module (install-local): a + * hook with no `body` ({@link collectHooksWithoutBody}) is withheld from + * `bindHooks` — warned, NOT bound — because on such a door its `handler` + * can never name the package's own code. A boot, which carries its runtime + * module, leaves this unset and binds such hooks as it always did. + */ + withholdHooksWithoutBody?: boolean; } /** What one {@link bindAppArtifactHandlers} call bound. */ @@ -119,6 +147,11 @@ export interface AppArtifactHandlerBinding { functions: number; /** Action handlers registered — one per bound declaration. */ actions: number; + /** + * [#21585] Hooks withheld under `withholdHooksWithoutBody`, by name — empty + * when the option is unset or every hook carries a `body`. + */ + withheldHooks: string[]; } /** @@ -138,7 +171,7 @@ export function bindAppArtifactHandlers( const { appId, logger } = options; const tag = `[${options.source ?? 'AppPlugin'}]`; const owner = appArtifactHandlerOwner(appId); - const out: AppArtifactHandlerBinding = { owner, hooks: 0, functions: 0, actions: 0 }; + const out: AppArtifactHandlerBinding = { owner, hooks: 0, functions: 0, actions: 0, withheldHooks: [] }; // ── Tear down the owner's previous set ────────────────────────────── try { @@ -151,9 +184,23 @@ export function bindAppArtifactHandlers( // ── Hooks (and the functions a hook may name) ─────────────────────── // Inline function handlers are resolved directly; string-named handlers // are looked up in `bundle.functions` (registered here too) or in any - // function previously registered on the engine. + // function previously registered on the engine — which is why a door with + // no runtime module withholds a hook that has no `body` (#21585). try { - const hooks = collectBundleHooks(bundle); + let hooks = collectBundleHooks(bundle); + if (options.withholdHooksWithoutBody) { + const withheld = hooksWithoutBodyOf(hooks); + if (withheld.length > 0) { + hooks = hooks.filter(hookCarriesBody); + out.withheldHooks = withheld.map((h) => h.name); + logger.warn( + `${tag} ${withheld.length} hook(s) with no \`body\` NOT bound: ${describeHooks(withheld)}. ` + + 'This door carries no runtime module, so a hook\'s `handler` can never name the package\'s own code. ' + + 'Give each hook a `body` (sandboxed JS) and install the package again.', + { appId, hooks: out.withheldHooks }, + ); + } + } // Entries, not bare handlers: each function's declared `effect` // (#4396) rides along to the registry, where a `script` node reads // it to report what its run actually did. @@ -238,21 +285,35 @@ export function bindAppArtifactHandlers( // ─── The job half (#21489) ───────────────────────────────────────────── /** - * An enabled job a JSON door cannot run: it carries no `body`. Its `handler` - * (deprecated) names a `defineStack({ functions })` entry — code, which a JSON - * artifact never carries (ADR-0088) — or it names nothing at all. + * An enabled job a JSON door cannot run: it carries no `body` — or, since + * #21585, a `body` that does not BIND (the declaration refuses it: an expression + * body, or one carrying `body.timeoutMs`), which no door can run either. "Without + * body" reads as "without a body that runs". Its `handler` (deprecated) names a + * `defineStack({ functions })` entry — code, which a JSON artifact never + * carries (ADR-0088) — or it names nothing at all. */ export interface JobWithoutBody { /** The job's `name`. */ name: string; /** The function name the job's `handler` declares, when it declares one. */ handler?: string; + /** + * [#21585] Set when the job HAS a `body` and it does not bind: the + * declaration's refusal ({@link judgeJobBody}). Absent for a job with no + * `body` at all. + */ + bodyRefusal?: string; } /** - * The enabled jobs of an artifact that carry no `body` — the jobs no JSON door - * can schedule (#21489). The install-local install route refuses a package - * that declares one; see the module header. + * The enabled jobs of an artifact that no JSON door can schedule (#21489): + * those with no `body`, and (#21585) those whose `body` does not BIND — an + * expression (L1) body, or one carrying `body.timeoutMs`, or any other shape + * the declaration refuses. That second half is the judgement the binder's own + * {@link jobBodyRunnerFactory} makes ({@link judgeJobBody}, a parse against + * `JobSchema.body`), so the door and the binder cannot disagree; such a job is + * named with its `bodyRefusal`. The install-local install route refuses a + * package that declares one; see the module header. * * Reads the jobs the binder reads ({@link collectBundleJobs}), and calls a job * enabled exactly when the binder does: `enabled: false` is the one value that @@ -263,15 +324,73 @@ export function collectJobsWithoutBody(bundle: unknown): JobWithoutBody[] { for (const job of collectBundleJobs(bundle)) { if (!job || typeof job !== 'object') continue; if (job.enabled === false) continue; - if (job.body) continue; + let bodyRefusal: string | undefined; + if (job.body) { + const judged = judgeJobBody(job.body); + if (judged.binds) continue; + bodyRefusal = judged.refusal; + } out.push({ name: typeof job.name === 'string' ? job.name : String(job.name), ...(typeof job.handler === 'string' ? { handler: job.handler } : {}), + ...(bodyRefusal !== undefined ? { bodyRefusal } : {}), }); } return out; } +// ─── Hooks with no `body` (#21585) ───────────────────────────────────── + +/** + * A hook with no `body`: its code is only a `handler` (deprecated) — a + * function name — or nothing at all. See the module header for why a door that + * carries no runtime module neither installs nor binds one. + */ +export interface HookWithoutBody { + /** The hook's `name`. */ + name: string; + /** The function name the hook's `handler` declares, when it declares one. */ + handler?: string; +} + +/** + * Does `hook` carry a `body` the engine's binder reads as one? The binder's own + * body-first test (`resolveHandler` in `@objectstack/objectql`'s hook binder): + * a `body` object is bound through the body runner and its `handler` is never + * consulted — whether or not the body then binds — while any other `body` value + * falls through to the `handler`. So exactly the hooks this answers `false` for + * are the ones whose `handler` the engine resolves by name. + */ +function hookCarriesBody(hook: any): boolean { + return Boolean(hook?.body) && typeof hook.body === 'object'; +} + +function hooksWithoutBodyOf(hooks: readonly any[]): HookWithoutBody[] { + return hooks.filter((h) => !hookCarriesBody(h)).map((h) => ({ + name: typeof h?.name === 'string' ? h.name : String(h?.name ?? h), + ...(typeof h?.handler === 'string' ? { handler: h.handler } : {}), + })); +} + +/** `'name' (handler 'fn')`, comma-joined — the same rendering the door's refusal uses. */ +function describeHooks(hooks: readonly HookWithoutBody[]): string { + return hooks.map((h) => `'${h.name}' (${h.handler !== undefined ? `handler '${h.handler}'` : 'no handler'})`).join(', '); +} + +/** + * The hooks of an artifact that carry no `body` (#21585) — every one of them: + * a hook has no on/off switch (`HookSchema` refuses `enabled` / `active`). The + * install-local install route refuses a package that declares one, and + * {@link bindAppArtifactHandlers} withholds them under + * `withholdHooksWithoutBody`; both read this judgement, over the hooks the + * binder reads ({@link collectBundleHooks}), so the door and the binder cannot + * disagree. A hook carrying both a `body` and a `handler` binds its `body` and + * is not named. + */ +export function collectHooksWithoutBody(bundle: unknown): HookWithoutBody[] { + return hooksWithoutBodyOf(collectBundleHooks(bundle)); +} + export interface AppArtifactJobSchedulingOptions { /** The app the jobs belong to: the artifact manifest's `id` (falling back to `name`). */ appId: string; diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts index 1d0a47cd96b..4efef8b99a7 100644 --- a/packages/runtime/src/index.ts +++ b/packages/runtime/src/index.ts @@ -69,14 +69,25 @@ export { AppPlugin, collectBundleHooks, collectBundleFunctions, collectBundleFun // install-local plugin (`@objectstack/cloud-connection`) on install and rehydrate. // [#21489] …and its job half: `scheduleAppArtifactJobs` schedules a package's // jobs (a `body` runs sandboxed on every door), and `collectJobsWithoutBody` -// names the enabled jobs no JSON door can run, which install-local refuses. -export { bindAppArtifactHandlers, appArtifactHandlerOwner, scheduleAppArtifactJobs, collectJobsWithoutBody } from './app-artifact-handlers.js'; +// names the enabled jobs no JSON door can run (no `body`, or one that does not +// bind), which install-local refuses. +// [#21585] `collectHooksWithoutBody` names the hooks whose code is only a +// function-name `handler`: install-local refuses them, and withholds them on a +// rehydrate (`withholdHooksWithoutBody`). +export { + bindAppArtifactHandlers, + appArtifactHandlerOwner, + scheduleAppArtifactJobs, + collectJobsWithoutBody, + collectHooksWithoutBody, +} from './app-artifact-handlers.js'; export type { AppArtifactHandlerBinding, AppArtifactHandlerBindingOptions, AppArtifactJobScheduling, AppArtifactJobSchedulingOptions, JobWithoutBody, + HookWithoutBody, } from './app-artifact-handlers.js'; // #14094 — what a DECLARATIVE job's handler is invoked with. A job has no graph, // so unlike a flow `script` node it is given data reach (`ql`) instead of being a diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index eadb9e70b1c..4a2653b2c52 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -44,8 +44,9 @@ * ({@link warnDiscardedRecordWrites}) instead of silent. */ -import type { Hook } from '@objectstack/spec/data'; -import { HookBodySchema, ScriptBodySchema } from '@objectstack/spec/data'; +import type { Hook, ScriptBodyParsed } from '@objectstack/spec/data'; +import { HookBodySchema } from '@objectstack/spec/data'; +import { JobSchema } from '@objectstack/spec/system'; import type { JobHandler, JobRunOutcome } from '@objectstack/spec/contracts'; import type { ScriptRunner, ScriptContext, ScriptResult, ScriptOrigin } from './script-runner.js'; // The record-title contract, imported rather than re-derived (#11293). The @@ -477,6 +478,43 @@ export function actionBodyRunnerFactory( }; } +/** What {@link judgeJobBody} answers for a `body` that binds, and for one that does not. */ +export type JobBodyJudgement = + | { binds: true; body: ScriptBodyParsed } + | { binds: false; refusal: string; issues: ReadonlyArray<{ path: PropertyKey[]; message: string }> }; + +/** + * [#21585] The ONE judgement whether a job's present `body` binds — the + * declaration's own: `raw` is parsed against `JobSchema.body` (the L2 script + * body shape, with `body.timeoutMs` refused on a job), so this answers exactly + * what `os validate` answers about the same slot. + * + * Two readers, so they cannot disagree: {@link jobBodyRunnerFactory} binds + * nothing for a body this refuses, and the install-local door refuses a + * package whose enabled job carries one (`collectJobsWithoutBody` in + * `../app-artifact-handlers.ts`). Before the door asked, it judged only that a + * `body` was PRESENT — an L1 expression body, or one carrying `timeoutMs`, + * installed with a 200 and the job was never scheduled, with only a server + * `warn` to say so. + * + * `refusal` is the declaration's first refusal sentence, prefixed with the + * key it names — the text the author acts on. Call it on a PRESENT body: an + * absent one is a different fact (the job has no `body`), which the slot's + * `.optional()` would answer as binding. + */ +export function judgeJobBody(raw: unknown): JobBodyJudgement { + const parsed = JobSchema.shape.body.safeParse(raw); + if (parsed.success && parsed.data !== undefined) return { binds: true, body: parsed.data }; + const issues = parsed.success ? [] : parsed.error.issues; + const first = issues[0]; + const at = first && first.path.length > 0 ? `body.${first.path.map(String).join('.')}: ` : ''; + return { + binds: false, + refusal: first ? `${at}${first.message}` : 'the job `body` is not a sandboxed JS body', + issues, + }; +} + /** * Job body runner factory (#21489) — the ONE point a job's `body` * (`JobSchema.body`: the hook body shape, L2 only) becomes the `JobHandler` @@ -503,8 +541,8 @@ export function actionBodyRunnerFactory( * The job's own `timeoutMs` reaches the runner as `opts.timeoutMs` — the ONE * limit of a body job (`JobSchema.timeoutMs`). `body.timeoutMs` is refused on a * job by the spec, so a body carrying one never parsed; it is refused here as - * well, rather than run under a second limit. A job with no `timeoutMs` gets - * the runner's job default. + * well ({@link judgeJobBody}), rather than run under a second limit. A job with + * no `timeoutMs` gets the runner's job default. * * ## What it resolves * @@ -527,24 +565,16 @@ export function jobBodyRunnerFactory( const raw = job.body; if (!raw) return undefined; - const parsed = ScriptBodySchema.safeParse(raw); - if (!parsed.success) { - opts.logger?.warn?.('[BodyRunner] invalid job.body shape — the job is NOT scheduled', { + const judged = judgeJobBody(raw); + if (!judged.binds) { + opts.logger?.warn?.(`[BodyRunner] invalid job.body shape — the job is NOT scheduled: ${judged.refusal}`, { appId: opts.appId, job: job.name, - issues: parsed.error.issues.slice(0, 3), + issues: judged.issues.slice(0, 3), }); return undefined; } - const body = parsed.data; - if (body.timeoutMs !== undefined) { - opts.logger?.warn?.( - `[BodyRunner] job '${job.name}' carries \`body.timeoutMs\`, which a job does not accept — the job is NOT ` - + "scheduled. A job's time limit is the job's own `timeoutMs`: move the value there (milliseconds, unchanged).", - { appId: opts.appId, job: job.name }, - ); - return undefined; - } + const body = judged.body; return async function boundJobHandler(): Promise { const sandboxCtx = buildJobSandboxContext(