Skip to content

fix(notebook): stop abandoned environment mutations - #2222

Open
wen2zhou wants to merge 1 commit into
aipoch:mainfrom
wen2zhou:codex/notebook-task-lifecycle
Open

fix(notebook): stop abandoned environment mutations#2222
wen2zhou wants to merge 1 commit into
aipoch:mainfrom
wen2zhou:codex/notebook-task-lifecycle

Conversation

@wen2zhou

@wen2zhou wen2zhou commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Problem

manage_environments and manage_packages could return an MCP timeout while the Micromamba process continued running in the Electron main process. Cancellation stopped at the MCP boundary: named-environment creation passed no signal, the network-sandbox defaultSpawn path had neither an abort signal nor a bounded deadline, and repeated mutations for the same environment waited behind the abandoned operation. In production logs this left Micromamba consuming a CPU core for more than 13 minutes; another install reported failure at 60 seconds and completed in the background at 78 seconds.

Real application reproduction also exposed three contributors to failed or excessively slow environment creation on Windows: a stale PATH-selected Micromamba could override the verified pinned runner, working-cache publication spawned a PowerShell ACL probe for every extracted directory, and USTC package downloads redirected to mirrors.nju.edu.cn, which the Notebook network policy rejected.

Proposed change

  • Carry the request AbortSignal from the MCP server through local RPC, environment creation, package installation, sandbox preparation, and the spawned installer process tree.
  • Give sandboxed package processes a bounded deadline, terminate the complete process tree on abort or timeout, and keep the recovery barrier when process exit cannot be confirmed.
  • Reject a concurrent caller-owned mutation for the same environment instead of queueing it behind the active mutation; make lease acquisition itself cancellable.
  • Emit MCP progress notifications while long environment and package operations are active for clients that extend their request deadline on progress.
  • Prefer the verified pinned Micromamba runner over stale PATH receipts.
  • Validate Windows working-cache ownership at traversal boundaries while retaining per-entry lstat, realpath containment, reparse-point, and digest checks.
  • Allow USTC's known mirrors.nju.edu.cn redirect target without allowing arbitrary redirected domains.

Scope and non-goals

This changes the product's Electron main-process Notebook lifecycle, shared mirror policy, and the Windows certification fixture. It is not a machine-specific configuration workaround and adds no renderer copy or UI behavior.

The Windows lane is included because process-tree termination, ACL validation, Micromamba selection, and the reported failure were Windows-specific. Linux and macOS keep the same lifecycle contract and unit coverage, but this machine could not run real Micromamba cancellation E2E on those platforms. Progress notifications help clients that reset deadlines on progress; cancellation and process termination remain the correctness mechanism for clients with a fixed hard deadline.

Acceptance criteria and validation

All commands below were run after the last material code edit at commit c6ae173573168ce36354ac1b1cd3330055625a82.

Behavior Command Result
An MCP cancellation reaches an active environment/package mutation, stops the installer tree, and a later request is not blocked by an abandoned same-environment operation npx vitest run src/main/notebook/mcp-management-lifecycle.integration.test.ts src/main/notebook/local-rpc-server.test.ts src/main/notebook/local-rpc-notebook-adapter.test.ts src/main/notebook/environment-management.test.ts src/main/notebook/environment-operations.test.ts Passed as part of the focused Notebook lifecycle run.
Sandbox aborts and deadlines terminate both root and descendant processes npx vitest run src/main/notebook/package-process-sandbox.test.ts 4 passed.
Micromamba selection, archive publication, provisioning, and package mutation remain valid on Windows npx vitest run src/main/notebook/windows-micromamba-runner.test.ts src/main/notebook/micromamba-archive-store.test.ts src/main/notebook/provisioner.test.ts src/main/notebook/package-manager.test.ts src/main/notebook/package-mutation.test.ts 171 passed.
USTC's known redirect works while Notebook network restrictions remain enforced npx vitest run src/shared/notebook-network.test.ts src/main/notebook/mirror-probe.test.ts src/main/notebook/network-sandbox-owner.test.ts 42 passed.
The packaged application can create a named environment, install a dependency, and execute it from that environment OPEN_SCIENCE_E2E_REAL_MICROMAMBA=1 node node_modules/@playwright/test/cli.js test e2e/certification/notebook-lifecycle.spec.ts --grep "creates and executes in a real" --workers=1 1 passed in 1.4 minutes using real Micromamba 2.8.1 and USTC downloads. The app created Python 3.12, installed python-docx, and imported it from the new e2e-real prefix.
Changed code meets repository formatting and lint rules npm run lint; npx prettier --check <changed .ts/.tsx/.js/.mjs/.json/.md files>; git diff --check 726e1fb1 HEAD Passed.
The repository's impact policy is exercised npm run test:affected -- -- --base 726e1fb1 --head HEAD Selected the full suite because the changed E2E fixtures have no mapped owner. The run exhausted Vitest worker memory after reporting unrelated failures; it did not complete cleanly. The renderer, managed-file-version, and storage IPC timeout failures passed when rerun individually (75, 46, and 123 tests respectively). Five Windows directory-symlink fixture failures and an intermittent working-file receipt rename EPERM were reproduced with the identical tests at baseline 726e1fb1; the affected tests and implementations are unchanged in this branch.
Node types remain unchanged outside this patch npm run typecheck:node Reports four TS1117 duplicate-property errors in unchanged src/main/delegation/production-composition.test.ts lines 962, 964, 965, and 972. No changed file reports a type error.

Review focus

Please focus on the ownership boundary between MCP requests and background mutations, especially abort propagation through local RPC and lease acquisition; the guarantees around killing and confirming the installer process tree; and the decision to preserve recovery state when termination cannot be confirmed. For the Windows fixes, please also review that reducing ACL subprocess calls does not weaken archive traversal checks, and that the added redirect allowance is limited to the USTC mirror's known NJU target.

@github-actions github-actions Bot added the bug Something isn't working label Sep 5, 2026
@wen2zhou
wen2zhou force-pushed the codex/notebook-task-lifecycle branch from c6ae173 to e7b3b79 Compare September 5, 2026 17:00
@wen2zhou
wen2zhou marked this pull request as ready for review September 5, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant