Skip to content

refactor: one Effect process layer for the CLI, Electron main and supervisor - #1070

Draft
zxch3n wants to merge 8 commits into
feat/effect-process-callersfrom
feat/shared-process-layer
Draft

zxch3n wants to merge 8 commits into
feat/effect-process-callersfrom
feat/shared-process-layer

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

Refs #429

Stack

  1. docs: propose Effect-scoped turn execution and lifecycle migration roadmap #1057: migration plan (docs), base main
  2. feat(cli): terminate ACP process trees through an Effect process layer #1065: L0 platform + L1 process-tree layer for ACP processes
  3. refactor(cli): route every CLI process through the Effect process layer #1069: every remaining CLI process caller, plus the boundary guard
  4. This PR: one process layer for the CLI, Electron main, the supervisor and shared Node helpers

Merge in order, and retarget the next PR to main after each merge.

Problem / pressure

After #1069 the CLI had a single process implementation. Four places outside apps/cli still spawned and killed processes directly with their own escalation loops:

  • Electron main (7 files);
  • packages/cli-supervisor's SIGTERM/SIGKILL loop;
  • packages/shared's git, CLI-detection and lock helpers, which the CLI itself calls;
  • the hand-maintained .cjs twins of those helpers.

These packages cannot import apps/cli, so the repository still had several implementations.

Summary

  • The layer moves into shared. It now lives in packages/shared/src/node/process.ts (@lody/shared/node/process):
    • It is a single module with no relative imports, because Electron's node --test --experimental-strip-types cannot resolve extensionless relative imports.
    • It carries the Promise facades and process-testing.ts (the fake process table).
    • apps/cli/src/platform keeps only the session containers, the logger bridge and a facade that adds the CLI logger.
  • The remaining users are migrated:
    • Electron main: the embedded CLI spawn and its bounded tree termination on quit, the protocol client, shell env, the updater's pkexec, the app icon, hang diagnostics and the path launcher.
    • The supervisor's escalation, which now goes through the tree and respects the launcher's processGroup.
    • The shared helpers cli-detection, local-project and file-lock.
  • The three .cjs twins are deleted. A repository search found only their own parity tests loading them. Their unique test cases were moved onto the TypeScript modules.
  • New facades:
    • probePid / probePidSync return three states. With them, file-lock keeps treating an EPERM (foreign-owned) pid as a dead owner, as before.
    • signalChildTreeNow is a synchronous signal for process.on('exit') handlers.
  • Guard and docs:
    • The guard covers apps/cli/src, apps/electron/src/main, packages/cli-supervisor/src and packages/shared/src/node (including .cjs). It now also flags <child>.kill(.
    • The rules moved next to the code, in packages/shared/src/node/AGENTS.md (with a CLAUDE.md symlink). apps/cli/AGENTS.md and apps/electron/src/AGENTS.md point to it.
  • Decisions and trade-offs are recorded in the process-tree-layer note, under "Follow-up: one layer for the whole repository".

Visual explanation

flowchart TD
    CLI["apps/cli (containers, facade + logger)"] --> P["@lody/shared/node/process"]
    E["apps/electron/src/main"] --> P
    S["packages/cli-supervisor"] --> P
    H["shared node helpers: local-project, cli-detection, file-lock"] --> P
    P --> OS["NodeProcess: the only child_process / kill access"]
    G["check:cli-process-boundary"] -.guards.-> CLI & E & S & H
Loading

Before / after

Before After
Process layer only inside apps/cli One module in packages/shared, used by CLI, Electron main, supervisor and shared helpers
Electron quit: its own SIGTERM → SIGKILL loop; Windows killed only the root Bounded tree termination; a survivor still fails quit
Supervisor escalation via child.kill Tree signal and termination through the layer
.cjs twins duplicating process logic Deleted (no runtime consumer)
The guard covered only apps/cli imports It covers four roots and direct child.kill

Test plan

  • corepack pnpm check passes end to end:
    • typecheck;
    • lint (0 errors);
    • CLI tests: 3200 passed, 4 skipped. The 19 core process tests moved to shared.
    • shared tests: 1259 passed;
    • supervisor tests: 52 passed;
    • Electron node --test: 195 passed;
    • components tests: 4373 passed;
    • i18n and the code-collab, platform, process and public-boundary guards.
  • New tests:
    • Supervisor, on the fake process table: SIGKILL escalation, SIGTERM fallback, group kill, and fatal-on-survivor. Each was checked against its regression.
    • signalChildTreeNow delivering its signal synchronously.
    • file-lock: it takes over a lock whose pid is foreign (pid 1) and keeps one held by our own live process.
    • cli-detection with a fake binary on PATH.
  • Loading @lody/shared/node/process and process-testing under plain node --experimental-strip-types works.
  • Not verified:
    • Windows hosts;
    • a packaged Electron build. Main bundles @lody/shared and effect, but no packaged app was built here.

Context handoff

The Lody team asked to finish removing the duplicate process implementations ("继续"), after #1069 had covered apps/cli.

Review fixes (9589a39)

A deep correctness review of the whole stack found regressions against the code it replaced. They are fixed here, each with a test that fails without its fix:

  • a failed spawn could SIGKILL the caller's own process group;
  • a timed-out git left index.lock behind;
  • a tree surviving SIGKILL hung runCommand in its finalizer;
  • macOS zombie EPERM gave a false TerminationFailed;
  • Windows ran a repository's git.cmd from cwd and lost ENOENT (unit-tested only, not run on Windows);
  • a forced Session.terminate waited behind graceful work;
  • archiving aborted when a tree survived;
  • the cgroup container allowed spawns after cleanup and missed nested cgroups.

Smaller changes, most without new tests:

  • the PTY waits 2 s after the hangup before SIGKILL;
  • the shell-env probe allows 15 s and no longer caches a failure;
  • rundll32 runs with a visible window state;
  • the supervisor no longer keeps every run alive through a never-settling promise;
  • process-layer warnings go to a real logger by default.

Details are in the process-tree-layer note. pnpm check passes when the session's injected GIT_CONFIG_* variables are removed; with them, one git-remote test fails for environmental reasons.

🤖 Generated with Claude Code

…ervisor

Move the process layer from apps/cli into packages/shared/src/node/process.ts
(@lody/shared/node/process), a single module with no relative imports so
Electron's node --test can load it, together with its Promise facades and the
fake process table. The CLI keeps its session containers and a facade that
only adds its logger.

Migrate the remaining direct process users: Electron main (CLI spawn and quit,
protocol client, shell env, updater, app icon, hang diagnostics, path launcher),
the CLI supervisor's SIGTERM/SIGKILL loop, and the shared cli-detection,
local-project and file-lock helpers. Delete their unused CommonJS twins, add a
three-state pid probe so file-lock keeps treating a foreign-owned pid as a dead
owner, and add signalChildTreeNow for exit handlers.

Extend the boundary guard to apps/electron/src/main, packages/cli-supervisor
and packages/shared/src/node (including .cjs) and to direct child kill calls;
move the process rules next to the code in packages/shared/src/node/AGENTS.md.

Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
zxch3n and others added 7 commits September 28, 2026 00:00
Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- childTree never signals a child without a pid: until Node reports a failed
  spawn, child.kill() reaches pid 0, the caller's own process group.
- Abandoned commands get a 2 s SIGTERM grace, so a timed-out git removes
  its index.lock instead of locking the repository.
- Waits inside finalizers are bounded by the clock (waitUntilGone) and the
  taskkill deadline completes its Deferred from a timer fiber, so a tree
  that survives SIGKILL can no longer hang runCommand forever.
- EPERM from a group signal waits instead of failing (macOS zombie leader).
- Windows commands resolve through absolute PATH entries only, never the
  working directory; an unresolvable one reports ENOENT via Node's spawn.
- Session.terminate escalates an in-flight graceful termination, does not
  wait on terminals when forced, and re-terminates processes started after
  a finished termination.
- Archive releases a Session even when a process tree survives.
- PTY: hangup, 2 s grace, SIGKILL. Shell-env probe: 15 s, failures not
  cached. rundll32 visible. cgroup: no spawn after cleanup, nested cgroups
  count as alive. Supervisor: no shared never-settling promise. Process
  warnings reach a real logger by default.

Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the CommandSpec.onSpawned hook main's Codex profile logout needs into
the shared process layer (with a test), and point main's new Codex profile
login test at the nodeProcess test seam.

Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Model: claude-opus-5-5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant