diff --git a/docs/development/task-startup.md b/docs/development/task-startup.md new file mode 100644 index 00000000..2020c57e --- /dev/null +++ b/docs/development/task-startup.md @@ -0,0 +1,54 @@ +# Task startup and notification loading + +`task` performs shell configuration, status reporting and dependency-path discovery +before invoking the selected runtime. These costs are separate from cron callback +latency and time spent waiting for an execution slot. + +## Dependency-path cache + +Node scheduling usually starts in `/ql`, while Alpine crond starts in `/root`. +The pnpm discovery key includes the working directory, npm/pnpm configuration, +relevant environment and executable metadata. Each key now has its own cached +record, so alternating scheduler environments do not evict each other. + +Records expire after 60 seconds. Refreshes share a persistent per-user lock; +records older than an hour are removed on refresh. Warm lookups do not scan the +cache directory. The lock is never removed while another process might use it. +The opt-out `QL_NODE_PATH_CACHE=0`, failed-discovery fallback, and package resolution +order remain available. This caches the dependency directory, not its contents. + +## Language startup paths + +| Invocation | Environment and before hooks | Automatic notification loading | +| --- | --- | --- | +| `task script.py` / `.pyc` | Python preload imports generated environment, runs the shell/command before hooks, imports `task_before.py`, then applies account selection | `QLAPI.notify(...)` imports `__ql_notify__` on first call | +| `task script.js` / `.mjs` / `.ts` | Node preload imports generated environment, runs shell/command before hooks, requires `task_before.js`, then applies account selection | `QLAPI.notify(...)` requires `__ql_notify__.js` on first call | +| `task script.sh` | Shell sources the generated environment and runs shell before hooks | No automatic Python/Node notification module | +| Explicit interpreter or other command, e.g. `task python3 script.py` | Existing generic-command path uses shell environment/before hooks; it does not automatically install the Python/Node preload | No automatic notification module unless configured by the caller | + +TypeScript uses `ts-node-transpile-only`; MJS uses the same Node preload plus the +ESM loader. There are no separate built-in Ruby, Go, Java or PHP preload modules. +All these task paths still pass through shared dependency-path setup: non-JS +scripts and their hooks may themselves call Node or use the QLAPI bridge. + +The before hooks are deliberately still synchronous: they can export environment +variables or perform work required by the user's script. Python's preload still +starts a child shell/Python to capture that environment; Node's preload starts a +child shell/Node. Node also still loads its gRPC client eagerly. These are remaining +startup costs, not work performed by the cron library, and are not removed by this +change. + +## Lazy notifications + +Scripts keep calling `QLAPI.notify` with the same arguments. When no notification +is sent, the notification provider and its channel dependencies are never imported +by the preload. First use loads the provider; subsequent uses reuse the standard +Python/Node module cache. JS forwards the receiver and return value, including +promises, and Python forwards positional and keyword arguments. Import/send errors +surface at the notification call rather than preventing unrelated scripts from +obtaining QLAPI during startup. A failed import can be retried after its cause is +resolved. + +Provider initialization, including provider configuration reads, now happens at +first use. A script's explicit `import __ql_notify__`, `require(...)`, or independent +notification helper still loads that module immediately. diff --git a/shell/node_path_cache.sh b/shell/node_path_cache.sh index 23363227..aeff640b 100644 --- a/shell/node_path_cache.sh +++ b/shell/node_path_cache.sh @@ -65,7 +65,9 @@ ql_read_node_path_cache() { # the caller's descriptors. A crashed refresher releases the kernel lock. ql_refresh_node_global_path() ( local cache="$1" key="$2" now result previous_umask - local lock="${cache}.lock" + # Keep one persistent lock per owner. Removing per-key lock files could + # split concurrent waiters across different inodes during cache cleanup. + local lock="${dir_tmp}/pnpm-root-${EUID}.lock" previous_umask=$(umask) umask 077 if type -P flock &>/dev/null && mkdir -p -- "$dir_tmp" 2>/dev/null; then @@ -93,6 +95,10 @@ ql_refresh_node_global_path() ( # Private lock creation must not change pnpm's inherited creation mask. umask "$previous_umask" + # Warm lookups never scan the directory. Retire old environment records on + # refresh, while preserving the shared lock and unrelated cache files. + find "$dir_tmp" -maxdepth 1 -type f -user "$EUID" \ + -name "pnpm-root-${EUID}-*.cache" -mmin +60 -delete 2>/dev/null || true now=${EPOCHSECONDS:-$(date +%s)} result=$(pnpm root -g 9>&- 2>/dev/null) || return $? # Never cache failed, empty, multiline or non-absolute answers. @@ -119,8 +125,10 @@ ql_get_node_global_path() { fi local key cache - cache="${dir_tmp}/pnpm-root-${EUID}.cache" key=$(ql_node_path_cache_key) || { pnpm root -g 2>/dev/null; return $?; } + # Node and crond may have different working directories/configuration. + # Preserve both discoveries instead of evicting each other on every run. + cache="${dir_tmp}/pnpm-root-${EUID}-${key// /-}.cache" if ql_read_node_path_cache "$cache" "$key"; then return 0 fi diff --git a/shell/preload/sitecustomize.js b/shell/preload/sitecustomize.js index 084c346e..b8d5bcbe 100644 --- a/shell/preload/sitecustomize.js +++ b/shell/preload/sitecustomize.js @@ -159,9 +159,13 @@ try { run(); - const { sendNotify } = require('./__ql_notify__.js'); global.QLAPI = { - notify: sendNotify, + notify(...args) { + // Keep notification dependencies out of ordinary script startup. + // require caches the module after the first notification. + const { sendNotify } = require('./__ql_notify__.js'); + return sendNotify.apply(this, args); + }, ...client, }; } catch (error) { diff --git a/shell/preload/sitecustomize.py b/shell/preload/sitecustomize.py index f4c51dd6..f0d94488 100644 --- a/shell/preload/sitecustomize.py +++ b/shell/preload/sitecustomize.py @@ -129,10 +129,12 @@ try: run() - from __ql_notify__ import send - class BaseApi(Client): def notify(self, *args, **kwargs): + # Most scripts never send a notification. Python caches this + # module after the first call, including its channel dependencies. + from __ql_notify__ import send + return send(*args, **kwargs) QLAPI = BaseApi() diff --git a/test/back/node-path-cache.test.cjs b/test/back/node-path-cache.test.cjs index 08dab052..06f272d2 100644 --- a/test/back/node-path-cache.test.cjs +++ b/test/back/node-path-cache.test.cjs @@ -65,6 +65,35 @@ test('config, environment, cwd, and executable changes invalidate discovery', (t f.run(); assert.equal(f.count(), before + 1); }); +test('alternating Node/crond working directories retain independent cached roots', (t) => { + const f = fixture(t); + fs.mkdirSync(path.join(f.root, 'child')); + fs.writeFileSync(path.join(f.root, 'child', '.npmrc'), 'global-dir=/child\n'); + for (let i = 0; i < 4; i++) { + assert.equal(f.run({ ANSWER: '/parent/modules' }).stdout.trim(), '/parent/modules'); + assert.equal( + f.run({ ANSWER: '/child/modules' }, 'cd child; ' + f.command).stdout.trim(), + '/child/modules', + ); + } + assert.equal(f.count(), 2, 'each working directory should discover only once'); + assert.equal(fs.readdirSync(f.env.dir_tmp).filter((x) => x.endsWith('.cache')).length, 2); +}); +test('refresh retires old per-environment records without removing locks or unrelated files', (t) => { + const f = fixture(t); + fs.mkdirSync(f.env.dir_tmp); + const prefix = `pnpm-root-${process.getuid()}`; + const names = [`${prefix}-1-2.cache`, `${prefix}.lock`, 'unrelated.cache']; + const old = new Date(Date.now() - 2 * 60 * 60 * 1000); + for (const name of names) { + const file = path.join(f.env.dir_tmp, name); + fs.writeFileSync(file, ''); + fs.utimesSync(file, old, old); + } + assert.equal(f.run().status, 0); + assert.equal(fs.existsSync(path.join(f.env.dir_tmp, names[0])), false); + for (const name of names.slice(1)) assert.ok(fs.existsSync(path.join(f.env.dir_tmp, name))); +}); test('expired or malformed records refresh and failed lookups are not cached', (t) => { const f = fixture(t); f.run(); diff --git a/test/back/node-path-lock.test.cjs b/test/back/node-path-lock.test.cjs index 94d03e32..36b409ef 100644 --- a/test/back/node-path-lock.test.cjs +++ b/test/back/node-path-lock.test.cjs @@ -51,7 +51,11 @@ function fixture(t) { fs.existsSync(env.CALLS) ? fs.readFileSync(env.CALLS, 'utf8').trim().split('\n').length : 0; - const cache = path.join(env.dir_tmp, `pnpm-root-${process.getuid()}.cache`); + const key = run({}, '. "$HELPER"; ql_node_path_cache_key').stdout.trim(); + const cache = path.join( + env.dir_tmp, + `pnpm-root-${process.getuid()}-${key.replace(' ', '-')}.cache`, + ); return { root, bin, @@ -61,7 +65,7 @@ function fixture(t) { asyncRun, count, cache, - lock: cache + '.lock', + lock: path.join(env.dir_tmp, `pnpm-root-${process.getuid()}.lock`), }; } function check(r, output = '/test/global/node_modules') { diff --git a/test/back/preload-lazy-notify.test.cjs b/test/back/preload-lazy-notify.test.cjs new file mode 100644 index 00000000..b4ab3533 --- /dev/null +++ b/test/back/preload-lazy-notify.test.cjs @@ -0,0 +1,168 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +function fixture(t, language) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ql-lazy-notify-')); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const extension = language === 'python' ? 'py' : 'js'; + fs.copyFileSync(`shell/preload/sitecustomize.${extension}`, path.join(root, `sitecustomize.${extension}`)); + fs.writeFileSync(path.join(root, `env.${extension}`), ''); + fs.writeFileSync(path.join(root, 'before.sh'), 'export FROM_SHELL=before\n'); + fs.writeFileSync(path.join(root, 'before.js'), 'process.env.FROM_LANGUAGE = "javascript";\n'); + fs.writeFileSync(path.join(root, 'task_before.py'), 'import os\nos.environ["FROM_LANGUAGE"] = "python"\n'); + fs.writeFileSync(path.join(root, 'client.js'), 'module.exports = { getEnvs: () => "client-ready" };\n'); + fs.copyFileSync('shell/preload/client.py', path.join(root, 'client.py')); + fs.copyFileSync('shell/preload/esm-loader.mjs', path.join(root, 'esm-loader.mjs')); + const env = { + ...process.env, + PYTHONPATH: root, + PREV_PYTHONPATH: '', + NODE_OPTIONS: '', + PREV_NODE_OPTIONS: '', + QL_NODE_GLOBAL_PATH: '', + file_task_before: path.join(root, 'before.sh'), + file_task_before_js: path.join(root, 'before.js'), + dir_scripts: root, + task_before: 'export FROM_COMMAND=command', + envParam: 'ACCOUNTS', + numParam: '2-3', + ACCOUNTS: 'first&second&third', + }; + const write = (name, text) => fs.writeFileSync(path.join(root, name), text); + const run = (body, extensionOverride = extension) => { + const entry = path.join(root, `entry.${extensionOverride}`); + write(path.basename(entry), body); + const python = language === 'python'; + const result = spawnSync(python ? 'python3' : process.execPath, + python ? [entry] : ['--require', path.join(root, 'sitecustomize.js'), entry], + { cwd: root, env, encoding: 'utf8', timeout: 15000 }); + assert.equal(result.status, 0, result.stderr + result.stdout); + return result.stdout; + }; + return { root, env, write, run }; +} + +for (const extension of ['js', 'mjs']) { + test(`${extension} startup preserves hooks, environment selection and client without importing notifications`, (t) => { + const f = fixture(t, 'javascript'); + f.write('__ql_notify__.js', 'throw Error("notification dependencies must not load at startup");'); + f.run(` + const assert = ${extension === 'mjs' ? '(await import("node:assert/strict")).default' : 'require("node:assert/strict")'}; + assert.equal(process.env.FROM_SHELL, 'before'); + assert.equal(process.env.FROM_LANGUAGE, 'javascript'); + assert.equal(process.env.FROM_COMMAND, 'command'); + assert.equal(process.env.ACCOUNTS, 'second&third'); + assert.equal(QLAPI.getEnvs(), 'client-ready'); + assert.equal(typeof QLAPI.notify, 'function'); + `, extension); + }); +} + +test('JavaScript first notification imports once and preserves arguments, this, promises and errors', (t) => { + const f = fixture(t, 'javascript'); + f.write('__ql_notify__.js', ` + global.notificationLoads = (global.notificationLoads || 0) + 1; + exports.sendNotify = function (...args) { + if (args[0] === 'fail') throw new Error('send failed'); + global.notificationResult = Promise.resolve({ args, receiver: this }); + return global.notificationResult; + }; + `); + f.run(` + const assert = require('node:assert/strict'); + assert.equal(global.notificationLoads, undefined); + (async () => { + const options = { channel: 'fixture' }; + const promise = QLAPI.notify('title', 'content', options); + assert.equal(promise, global.notificationResult); + const result = await promise; + assert.deepEqual(result.args, ['title', 'content', options]); + assert.equal(result.receiver, QLAPI); + await QLAPI.notify('second'); + assert.equal(global.notificationLoads, 1); + assert.throws(() => QLAPI.notify('fail'), /send failed/); + })().catch(error => { console.error(error); process.exitCode = 1; }); + `); +}); + +test('JavaScript reports import failures on notify and permits a later retry', (t) => { + const f = fixture(t, 'javascript'); + f.write('__ql_notify__.js', ` + if (!process.env.NOTIFY_READY) throw Error('notification dependency unavailable'); + exports.sendNotify = (...args) => args; + `); + f.run(` + const assert = require('node:assert/strict'); + assert.equal(QLAPI.getEnvs(), 'client-ready'); + assert.throws(() => QLAPI.notify('first'), /notification dependency unavailable/); + process.env.NOTIFY_READY = '1'; + assert.deepEqual(QLAPI.notify('retry'), ['retry']); + `); +}); + +test('Python startup preserves hooks, environment selection and client without importing notifications', (t) => { + const f = fixture(t, 'python'); + f.write('__ql_notify__.py', 'raise RuntimeError("notification dependencies must not load at startup")\n'); + f.run(` +import os, sys +assert os.environ['FROM_SHELL'] == 'before' +assert os.environ['FROM_LANGUAGE'] == 'python' +assert os.environ['FROM_COMMAND'] == 'command' +assert os.environ['ACCOUNTS'] == 'second&third' +assert '__ql_notify__' not in sys.modules +assert callable(QLAPI.notify) +assert callable(QLAPI.getEnvs) +`); +}); + +test('Python first notification imports once and preserves args, kwargs, return values and errors', (t) => { + const f = fixture(t, 'python'); + f.write('__ql_notify__.py', ` +import builtins +builtins.notification_loads = getattr(builtins, 'notification_loads', 0) + 1 +def send(*args, **kwargs): + if args[0] == 'fail': + raise ValueError('send failed') + return args, kwargs +`); + f.run(` +import builtins, sys +assert '__ql_notify__' not in sys.modules +assert QLAPI.notify('title', 'content', channel='fixture') == (('title', 'content'), {'channel': 'fixture'}) +assert QLAPI.notify('second') == (('second',), {}) +assert builtins.notification_loads == 1 +try: + QLAPI.notify('fail') +except ValueError as error: + assert str(error) == 'send failed' +else: + raise AssertionError('notification exception was swallowed') +`); +}); + +test('Python reports import failures on notify and permits a later retry', (t) => { + const f = fixture(t, 'python'); + f.write('__ql_notify__.py', ` +import os +if not os.environ.get('NOTIFY_READY'): + raise RuntimeError('notification dependency unavailable') +def send(*args, **kwargs): + return args +`); + f.run(` +import os, sys +assert '__ql_notify__' not in sys.modules +try: + QLAPI.notify('first') +except RuntimeError as error: + assert str(error) == 'notification dependency unavailable' +else: + raise AssertionError('import exception was swallowed') +os.environ['NOTIFY_READY'] = '1' +assert QLAPI.notify('retry') == ('retry',) +`); +});