feat(proc): launch boundary + subprocess metering (kz8.2)#6
Merged
Conversation
Add src/proc/launch.ts as the single boundary for child_process in the Node runtime. Every spawn site will route through `launch()` / `launchSync()` and tag itself with a closed-union `LaunchCategory`. RuntimeStats owns the metering: total / inFlight / lastMinute (60s rolling window) / per-category counts + reservoir p50/p99 duration histograms. src/proc/stats-handle.ts is the DI seam: launch.ts imports the type; the daemon installs an implementation backed by RuntimeStats at startup via setLaunchStats(). Other runtimes (Node fallback, install) leave it null and pay no metering cost. [LAW:locality-or-seam] First migration: the two click-verb spawnSync calls in src/daemon/server.ts (pbcopy, open) now go through launchSync with category tags. server.ts no longer imports child_process. Remaining sites (git, tmux, terminal-width, var-system shell+git, install, daemon-acquire) follow in subsequent commits. daemon-stats output now includes a `subprocesses` block listing the non-zero categories with their counts and p50/p99 timings. [LAW:single-enforcer] [LAW:one-type-per-behavior] [LAW:dataflow-not-control-flow]
Route every child_process / Command::new through the launch boundary.
After this commit:
- grep -rn 'from .node:child_process.\|from .child_process.' src/ outside
src/proc/ → empty.
- grep -n 'Command::new' rust-client/src/main.rs → only descriptive comments.
TypeScript migrations:
- src/segments/tmux.ts → launch(category: "tmux")
- src/segments/git.ts: execGitAsync now parses the command string and calls
launch(category: "git") with bin="git" + args. All 12 call sites unchanged.
- src/utils/terminal-width.ts → launchSync(category: "terminal-width") for
ps / stty / tput / mode con. (kz8.4 will eliminate this fanout entirely.)
- src/var-system/sources.ts: execShell → launch("user-shell"),
execGit → launch("var-system.git").
- src/install/index.ts: osacompile / plutil / lsregister / pbcopy / open
→ launchSync with install.* categories. Throw-on-error preserved.
- src/daemon/acquire.ts: spawnDaemonDetachedReal now calls
launch(detached: true, category: "daemon-spawn"). Same sync semantics
(detached path returns inside the async body before any await).
Rust:
- rust-client/src/launch.rs (new) owns std::process::Command. Exposes
exec_node_replace and spawn_node_detached_daemon — two functions because
exec-replace and detached-fork are distinct acts, not flags.
[LAW:one-type-per-behavior]
- main.rs spawn sites collapsed to launch:: calls.
ESLint:
- no-restricted-imports rule bans child_process / node:child_process in
src/** except src/proc/**. A future regression fails lint, not just a
manual grep. [LAW:single-enforcer]
[LAW:single-enforcer] [LAW:dataflow-not-control-flow] [LAW:one-source-of-truth]
There was a problem hiding this comment.
Pull request overview
Introduces a single subprocess-launch boundary in the Node runtime (src/proc/launch.ts) and mirrors it in the Rust client (rust-client/src/launch.rs). All existing child_process/Command::new call sites are migrated to route through this boundary, and an ESLint rule forbids future direct imports. The Node primitive additionally feeds a RuntimeStats metering layer (counts, in-flight, rolling 60s window, per-category reservoir histograms) surfaced via cc-candybar daemon-stats --json and the human-readable formatter.
Changes:
- Add
launch/launchSync/setLaunchStatsprimitive with a closedLaunchCategoryunion and aLaunchStatsHandleseam toRuntimeStats. - Migrate every spawn site (daemon, segments, var-system, install, acquire, rust client) to the new primitive and enforce via
no-restricted-imports. - Extend
StatsSnapshot/formatter to expose subprocess metering (total, inFlight, lastMinute, byCategory, p50/p99 durations).
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/proc/launch.ts | New single Node spawn boundary with sync/async/detached paths and metering hooks. |
| src/proc/stats-handle.ts | Defines the seam interface between launch and RuntimeStats. |
| src/daemon/stats.ts | Adds subprocess counters, ring-buffer rolling window, per-category histograms, and snapshot fields. |
| src/daemon/client-stats.ts | Renders subprocesses block in the human-readable formatter. |
| src/daemon/server.ts | Installs setLaunchStats and migrates pbcopy/open click handlers to launchSync. |
| src/daemon/acquire.ts | Migrates daemon detached spawn through launch. |
| src/segments/tmux.ts | Migrates tmux display-message to launch. |
| src/segments/git.ts | Routes all 12 git invocations through launch via execGitAsync. |
| src/segments/sources.ts → src/var-system/sources.ts | Replaces inline spawn wrappers with launch. |
| src/utils/terminal-width.ts | Replaces execSync ps/stty/tput/mode con calls with launchSync. |
| src/install/index.ts | Migrates osacompile/plutil/lsregister/pbcopy/open install path through launchSync. |
| rust-client/src/launch.rs | New Rust launch boundary exposing exec_node_replace and spawn_node_detached_daemon. |
| rust-client/src/main.rs | Removes direct Command/CommandExt imports and routes through launch::. |
| eslint.config.mjs | Adds no-restricted-imports rule forbidding child_process outside src/proc/**. |
| test/proc/launch.test.ts | New tests for launch/launchSync covering success, non-zero, spawn-error, timeout, stdin, detached, metering. |
| test/daemon-stats.test.ts | Updates fixture stats with new subprocesses block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+206
to
+254
| const osa = launchSync({ | ||
| bin: "/usr/bin/osacompile", | ||
| args: [ | ||
| "-o", | ||
| bundle, | ||
| "-e", | ||
| appleScriptSource(process.execPath, stableScript, nodeModules), | ||
| ], | ||
| { stdio: ["ignore", "inherit", "inherit"] }, | ||
| ); | ||
| category: "install.plutil", | ||
| }); | ||
| if (!osa.ok) { | ||
| process.stderr.write(osa.stderr); | ||
| throw new Error(`osacompile failed (${osa.reason})`); | ||
| } | ||
|
|
||
| const plistPath = path.join(bundle, "Contents", "Info.plist"); | ||
|
|
||
| for (const { key } of infoPlistPatch()) { | ||
| // plutil errors if the key already exists; pre-delete so the operation is | ||
| // idempotent. Ignore failures (key may not exist on a fresh build). | ||
| spawnSync("/usr/bin/plutil", ["-remove", key, plistPath], { | ||
| stdio: "ignore", | ||
| launchSync({ | ||
| bin: "/usr/bin/plutil", | ||
| args: ["-remove", key, plistPath], | ||
| category: "install.plutil", | ||
| }); | ||
| } | ||
|
|
||
| for (const { key, xml } of infoPlistPatch()) { | ||
| execFileSync("/usr/bin/plutil", ["-insert", key, "-xml", xml, plistPath], { | ||
| stdio: ["ignore", "inherit", "inherit"], | ||
| const r = launchSync({ | ||
| bin: "/usr/bin/plutil", | ||
| args: ["-insert", key, "-xml", xml, plistPath], | ||
| category: "install.plutil", | ||
| }); | ||
| if (!r.ok) { | ||
| process.stderr.write(r.stderr); | ||
| throw new Error(`plutil -insert ${key} failed (${r.reason})`); | ||
| } | ||
| } | ||
|
|
||
| process.stdout.write(`Registering ${URL_SCHEME}:// with Launch Services\n`); | ||
| execFileSync( | ||
| "/System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister", | ||
| ["-f", bundle], | ||
| { stdio: ["ignore", "inherit", "inherit"] }, | ||
| ); | ||
| const lsr = launchSync({ | ||
| bin: "/System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister", | ||
| args: ["-f", bundle], | ||
| category: "install.plutil", | ||
| }); | ||
| if (!lsr.ok) { | ||
| process.stderr.write(lsr.stderr); | ||
| throw new Error(`lsregister failed (${lsr.reason})`); | ||
| } |
Comment on lines
+136
to
+224
| child.on("close", (code, signal) => { | ||
| if (code === 0) { | ||
| settle({ ok: true, stdout, stderr, exitCode: code }); | ||
| } else { | ||
| settle({ | ||
| ok: false, | ||
| reason: signal ? "timeout" : "non-zero", | ||
| stdout, | ||
| stderr, | ||
| exitCode: code, | ||
| signal, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| if (opts.timeoutMs && opts.timeoutMs > 0) { | ||
| timer = setTimeout(() => { | ||
| child.kill("SIGTERM"); | ||
| settle({ | ||
| ok: false, | ||
| reason: "timeout", | ||
| stdout, | ||
| stderr, | ||
| exitCode: null, | ||
| signal: "SIGTERM", | ||
| }); | ||
| }, opts.timeoutMs); | ||
| } | ||
|
|
||
| if (opts.stdinInput !== undefined && child.stdin) { | ||
| child.stdin.end(opts.stdinInput); | ||
| } else if (child.stdin) { | ||
| child.stdin.end(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // Sync variant. Only for callers that genuinely cannot be async — currently | ||
| // just `src/utils/terminal-width.ts`, called from the synchronous render-path. | ||
| // kz8.4 deletes those sites; once gone, this function can go too. | ||
| export function launchSync(opts: LaunchOpts): LaunchResult { | ||
| const t0 = Date.now(); | ||
| statsHandle?.onStart(opts.category); | ||
|
|
||
| const stdio: StdioOptions = ["pipe", "pipe", "pipe"]; | ||
| try { | ||
| const result = spawnSync(opts.bin, opts.args ?? [], { | ||
| cwd: opts.cwd, | ||
| env: opts.env, | ||
| input: opts.stdinInput, | ||
| timeout: | ||
| opts.timeoutMs && opts.timeoutMs > 0 ? opts.timeoutMs : undefined, | ||
| stdio, | ||
| encoding: "utf8", | ||
| }); | ||
|
|
||
| statsHandle?.onEnd(opts.category, Date.now() - t0); | ||
|
|
||
| const stdout = typeof result.stdout === "string" ? result.stdout : ""; | ||
| const stderr = typeof result.stderr === "string" ? result.stderr : ""; | ||
|
|
||
| if (result.error) { | ||
| // Node sets `error` for ETIMEDOUT and ENOENT; distinguish by code. | ||
| const code = (result.error as NodeJS.ErrnoException).code; | ||
| const reason: "timeout" | "spawn-error" = | ||
| code === "ETIMEDOUT" ? "timeout" : "spawn-error"; | ||
| return { | ||
| ok: false, | ||
| reason, | ||
| stdout, | ||
| stderr, | ||
| exitCode: null, | ||
| signal: result.signal ?? null, | ||
| error: result.error.message, | ||
| }; | ||
| } | ||
|
|
||
| if (result.status === 0) { | ||
| return { ok: true, stdout, stderr, exitCode: result.status }; | ||
| } | ||
|
|
||
| return { | ||
| ok: false, | ||
| reason: result.signal ? "timeout" : "non-zero", | ||
| stdout, | ||
| stderr, | ||
| exitCode: result.status, | ||
| signal: result.signal ?? null, | ||
| }; |
Comment on lines
397
to
410
| function spawnDaemonDetachedReal(): boolean { | ||
| const node = process.execPath; | ||
| const script = process.argv[1]; | ||
| if (!script) return false; | ||
| const child = spawn(node, ["--max-old-space-size=400", script, "daemon"], { | ||
| // Detached launches return synchronously after the spawn succeeds; the | ||
| // Promise resolves immediately (the OS now owns the process). | ||
| void launch({ | ||
| bin: node, | ||
| args: ["--max-old-space-size=400", script, "daemon"], | ||
| detached: true, | ||
| stdio: "ignore", | ||
| env: process.env, | ||
| category: "daemon-spawn", | ||
| }); | ||
| child.unref(); | ||
| return true; | ||
| } |
Comment on lines
+113
to
+123
| private recordRollingNow(): void { | ||
| const now = Date.now(); | ||
| if (this.rollingTimestamps.length < ROLLING_BUFFER_CAP) { | ||
| this.rollingTimestamps.push(now); | ||
| return; | ||
| } | ||
| // Ring buffer once cap hit. Old entries get overwritten; "last minute" | ||
| // truth is preserved because eviction filters by timestamp on read. | ||
| this.rollingTimestamps[this.rollingHead] = now; | ||
| this.rollingHead = (this.rollingHead + 1) % ROLLING_BUFFER_CAP; | ||
| } |
Comment on lines
40
to
66
| private async execGitAsync( | ||
| command: string, | ||
| options: { cwd: string; encoding: BufferEncoding; timeout: number }, | ||
| ): Promise<{ stdout: string }> { | ||
| return execAsync(command, { | ||
| ...options, | ||
| const parts = command.split(/\s+/); | ||
| const bin = parts[0]; | ||
| if (!bin || bin !== "git") { | ||
| throw new Error( | ||
| `execGitAsync expects "git ..." command, got: ${command}`, | ||
| ); | ||
| } | ||
| const args = parts.slice(1); | ||
| const result = await launch({ | ||
| bin, | ||
| args, | ||
| cwd: options.cwd, | ||
| env: { ...process.env, GIT_OPTIONAL_LOCKS: "0" }, | ||
| timeoutMs: options.timeout, | ||
| category: "git", | ||
| }); | ||
| if (!result.ok) { | ||
| throw new Error( | ||
| `git ${args.join(" ")} failed (${result.reason}, exit ${result.exitCode ?? "null"})`, | ||
| ); | ||
| } | ||
| return { stdout: result.stdout }; | ||
| } |
Comment on lines
+144
to
+152
| for (const [cat, n] of activeCats) { | ||
| const p50 = s.subprocesses.p50DurationMs[cat]; | ||
| const p99 = s.subprocesses.p99DurationMs[cat]; | ||
| const timing = | ||
| p50 !== undefined && p99 !== undefined | ||
| ? ` (p50 ${p50}ms · p99 ${p99}ms)` | ||
| : ""; | ||
| lines.push(` ${cat.padEnd(13)} ${n}${timing}`); | ||
| } |
Comment on lines
+148
to
+167
| const byCategory: Record<string, number> = {}; | ||
| for (const cat of LAUNCH_CATEGORIES) { | ||
| byCategory[cat] = this.subprocessCount.get(cat) ?? 0; | ||
| } | ||
|
|
||
| const p50: Record<string, number> = {}; | ||
| const p99: Record<string, number> = {}; | ||
| for (const [cat, hist] of this.subprocessHistogram) { | ||
| if (hist.length === 0) continue; | ||
| const sorted = [...hist].sort((a, b) => a - b); | ||
| const p50idx = Math.floor(sorted.length * 0.5); | ||
| const p99idx = Math.min( | ||
| sorted.length - 1, | ||
| Math.floor(sorted.length * 0.99), | ||
| ); | ||
| const p50val = sorted[p50idx]; | ||
| const p99val = sorted[p99idx]; | ||
| if (p50val !== undefined) p50[cat] = p50val; | ||
| if (p99val !== undefined) p99[cat] = p99val; | ||
| } |
Comment on lines
+151
to
+163
| if (opts.timeoutMs && opts.timeoutMs > 0) { | ||
| timer = setTimeout(() => { | ||
| child.kill("SIGTERM"); | ||
| settle({ | ||
| ok: false, | ||
| reason: "timeout", | ||
| stdout, | ||
| stderr, | ||
| exitCode: null, | ||
| signal: "SIGTERM", | ||
| }); | ||
| }, opts.timeoutMs); | ||
| } |
6 tasks
brandon-fryslie
added a commit
that referenced
this pull request
May 16, 2026
Eight findings from Copilot's review of #6, addressed in one PR: 1. install.osacompile / install.lsregister now have their own LAUNCH_CATEGORIES entries; per-binary metering no longer collapses three distinct install helpers into install.plutil. 2. launch / launchSync distinguish "timeout" (our local timer fired) from "signal" (OS/external killer ended the child). A signal-killed no-timeout child no longer misreports as timeout in daemon-stats. 3. launchDetachedSync is now exported and returns the typed outcome synchronously. spawnDaemonDetachedReal uses it and returns result.ok, so ENOENT / EACCES on detached daemon spawns surfaces instead of being reported as success. As a bonus, launchDetachedSync now detects ENOENT (which spawn doesn't throw for — it surfaces as pid=undefined). 4. Rolling-window ring buffer cap bumped to 16384 (~273 sustained launches/sec before undercount kicks in); cap rationale documented at the constant. 5. execGitAsync takes args: string[] directly; the prior whitespace-split contract is gone. Paths with spaces / quoted args can no longer mis-tokenize at the boundary. 6. byCategory now only includes executed categories — symmetric with p50/p99 histograms and with the formatter's filter. Consumers wanting the full enum read LAUNCH_CATEGORIES. 7. Stats formatter column width is computed from active categories (max(13, ...lengths)) instead of a hardcoded padEnd(13) that misaligned for var-system.git, install.plutil, terminal-width, and others. 8. Timer's child.kill is guarded by !settled so a very-fast child that exited before setTimeout was scheduled doesn't get a stray SIGTERM. [LAW:single-enforcer] [LAW:one-type-per-behavior] [LAW:no-silent-fallbacks] [LAW:dataflow-not-control-flow] [LAW:types-are-the-program] Refs: brandon-process-discipline-kz8.2.1
brandon-fryslie
added a commit
that referenced
this pull request
Jun 10, 2026
…caller values (6gd) (#101) Sheriff finding #6: the connect-with-timeout + frame round-trip was implemented three times (client.ts 50/150ms, client-stats.ts and client-debug.ts 200/500ms), and the stats/debug copies had drifted from client.ts's outcome classification. [LAW:one-type-per-behavior] src/daemon/client-transport.ts is now the single implementation: connect, one framed round-trip, socket teardown, and the transient/permanent failure classification (moved verbatim from client.ts, the maintained post-2l6 copy). The three clients pass their timeout budgets and payload projection as VALUES — render/click keep 50/150/200, stats/debug keep 200/500. No flags, no modes. [LAW:single-enforcer] PROTOCOL_VERSION is stamped on every outbound request inside the transport; callers cannot mis-stamp it. ClientOutcome becomes RoundTripOutcome<string>; stats/debug inherit the typed classification, so the "daemon may not be running" hint now appears only on transient failures (on permanent ones the daemon is demonstrably up). check-protocol anchors (CONNECT_TIMEOUT_MS / TOTAL_BUDGET_MS in client.ts) stay in place; all 12 mirrored constants verified. Co-authored-by: elton-prawn <264370887+elton-prawn@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/proc/launch.tsis the single boundary forchild_processin the Node runtime. Every spawn site routes throughlaunch()/launchSync()and tags itself with a closed-unionLaunchCategory.RuntimeStatsowns the metering: total / inFlight / lastMinute (60s rolling window) / per-category counts + reservoir p50/p99 duration histograms. Exposed incc-candybar daemon-stats --json.rust-client/src/launch.rsmirrors the boundary in Rust with two functions (exec_node_replace,spawn_node_detached_daemon) — two functions because exec-replace and detached-fork are distinct acts, not flags. No metering in Rust (single-frame, short-lived).no-restricted-importsrule banschild_processimports outsidesrc/proc/**. Future regressions fail lint, not just a manual grep.Migrated spawn sites
src/daemon/server.ts(pbcopy, open) —click.pbcopy,click.opensrc/daemon/acquire.ts(daemon-spawn) —daemon-spawnsrc/segments/tmux.ts—tmuxsrc/segments/git.ts(12 git invocations) —gitsrc/utils/terminal-width.ts(ps / stty / tput / mode con) —terminal-width(kz8.4 deletes these)src/var-system/sources.ts(execShell, execGit) —user-shell,var-system.gitsrc/install/index.ts(osacompile, plutil×N, lsregister, pbcopy, open) —install.*rust-client/src/main.rs(exec_node_fallback, spawn_daemon_detached) — both vialaunch::Definition of done (from ticket)
RuntimeStatsexposes per-category subprocess counts and durations.daemon-stats --jsonincludessubprocessesblock.grep -rn 'child_process\|Command::new'outside the launch module's file = empty in both trees.RuntimeStatsshow launches route through the primitive (test/proc/launch.test.ts).subprocesses.lastMinute == 0— verifiable post-merge by running the daemon and checkingcc-candybar daemon-stats --json.Test plan
pnpm typecheckcleanpnpm run lintclean (0 errors, 1 pre-existing warning)pnpm test— 32 suites / 679 tests passcargo build(rust-client) cleancc-candybar daemon-stats --jsonshows non-zerosubprocesses.byCategory.git[LAW:single-enforcer] [LAW:one-type-per-behavior] [LAW:dataflow-not-control-flow] [LAW:one-source-of-truth] [LAW:locality-or-seam]
Closes brandon-process-discipline-kz8.2.