fix: reduce task startup overhead and lazy-load notifications (#3080)

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.

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

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) {

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()

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();

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') {

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',)
+`);
+});