Bound and cancel the deepnote-toolkit probe

[tkislan]

Context

DeepnoteToolkitDependencyService.probe runs the toolkit probe on the user's interpreter with no cancellation and no time bound:

// src/kernels/deepnote/deepnoteToolkitDependencyService.node.ts:208
private async probe(interpreter: PythonEnvironment): Promise<ToolkitState> {
    try {
        const python = await this.pythonExecutionFactory.createActivatedEnvironment({ interpreter });
        const result = await python.exec(['-c', TOOLKIT_PROBE], { throwOnStdErr: false });
        ...
    } catch (error) {
        logger.warn(`Could not probe deepnote-toolkit in ${getDisplayPath(interpreter.uri)}`, error);

        return (await this.installer.isInstalled(Product.deepnoteToolkit, interpreter)) ? 'ok' : 'missing';
    }
}

checkAndInstall already has a CancellationToken and calls probe(interpreter) without it.

Why this matters

ensureToolkitInstalled wraps the shared work in raceCancellation(token, cancel, pending), so a caller whose notebook closes is released even if the probe hangs. That covers the common case, and it is why this is not a startup-blocking bug. What it does not cover:

1. Nothing cancels. On a wedged interpreter — a sitecustomize/usercustomize that blocks, an NFS or network-mounted venv that stalls, a conda activation hook waiting on a lock — probe stays pending forever and ensureToolkitInstalled never resolves. The catch is never reached, so the installer.isInstalled fallback the method's own docstring promises never runs.

2. The process leaks. On cancellation the caller is freed, but pending keeps running and the spawned Python is never killed. ProcessService kills a child only via options.token, which probe does not pass.

3. The hang is sticky per interpreter. pendingChecks clears its entry in .finally(), so a probe that never settles pins that cache entry for the rest of the session. Every later ensureToolkitInstalled for the same interpreter joins the same dead promise.

What the APIs support

Checked against the current tree:

• python.exec(args, options) — token supported. SpawnOptions is ChildProcessSpawnOptions & { encoding?, token?: CancellationToken, mergeStdOutErr?, throwOnStdErr? }, and it is honoured rather than decorative — proc.node.ts:180 registers options.token.onCancellationRequested(disposable.dispose), where dispose calls ProcessService.kill(proc.pid, killGroup). Passing a token kills the child, it does not merely settle the promise.

• SpawnOptions.timeout — supported. SpawnOptions extends Node's child_process.SpawnOptions, which carries timeout?: number, and ProcessService.exec passes options straight to spawn via getDefaultOptions. Node kills the child on expiry. This is the part that covers case 1, which a token alone does not.

• createActivatedEnvironment(options) — no token. ExecutionFactoryCreateWithEnvironmentOptions is { resource?, interpreter, allowEnvironmentFetchExceptions? }. Environment activation can itself shell out, so it is a second unbounded await, and the only lever from outside is to race it — which frees the caller without killing what it started.

Proposal

• Thread the token through: probe(interpreter, token), called from checkAndInstall with the token it already holds.
• python.exec(['-c', TOOLKIT_PROBE], { throwOnStdErr: false, token, timeout: PROBE_TIMEOUT_MS }), with the timeout as a named constant near the top of the module.
• Decide what to do about createActivatedEnvironment. Racing it against the token releases the caller but leaks whatever activation started; leaving it is honest but keeps one unbounded await. Worth deciding explicitly rather than by omission.

Acceptance

• A probe on a non-terminating interpreter fails within the timeout and falls through to installer.isInstalled, rather than pending forever.
• Cancelling the caller kills the probe process instead of orphaning it.
• A hung probe does not pin the pendingChecks entry for the session.
• A regression test that fails against today's code: a probe that never resolves, asserted to settle and to reach the fallback.

Origin

Raised by CodeRabbit on #513 (#513 (comment)). Its stated consequence — "notebook startup remains blocked" — is not accurate, because of the raceCancellation above; the underlying concern is. probe landed in #512 and is already on main, so #513 is not the place to fix it.