[juliusmarminge]

Problem

apps/desktop/scripts/dev-electron.mjs, the desktop dev supervisor, only stopped on SIGINT, SIGTERM or SIGHUP. If its parent was killed without forwarding a signal (kill -9, a crash, a terminal that died first), the supervisor was reparented and kept running. It relaunched Electron on every rebuild and crash, and later vp run dev:desktop runs ended up with several supervisors on the same T3 home, closing each other's windows.

Repro on Linux (Xvfb), main 8d3b061: start vp run dev:desktop --home-dir <dev home> and kill -9 the vp pack --watch process. The supervisor stays alive, reparented to the user's subreaper, and touch apps/desktop/dist-electron/main.cjs starts a new Electron. After a second vp run dev:desktop, two node scripts/dev-electron.mjs processes run at once.

Change

• dev-electron.mjs saves process.ppid at startup and checks it once a second with an unref'd timer. When the value changes, the supervisor runs its normal shutdown(129), which stops Electron and its child tree, then exits. The check compares against the saved value instead of 1 because on Linux orphans go to a subreaper. It is skipped on Windows, which never reparents.
• apps/desktop/vite.config.ts changes the tsdown onSuccess command to exec node scripts/dev-electron.mjs on non-Windows hosts. tsdown runs onSuccess through /bin/sh -c. On Debian and Ubuntu that shell is dash, which forks rather than exec'ing, so a resident sh stayed between vp pack and the supervisor and hid the parent's death. With exec, the supervisor is a direct child of vp pack. tsdown's tree-kill on rebuild still sends it SIGTERM.

Scope and approval

Fixes #19109
GitCafe: https://git.cafe/pingdotgg/t3code/issues/19109
Triaged bug via the automated issue flow; a maintainer confirmed it on the issue and labelled it accepted.

Verification

• vp test run apps/desktop/scripts/electron-launcher.test.mjs: 8 passed. The new test, dev supervisor stops Electron and exits once its parent process goes away, runs the real dev-electron.mjs startup with a fake clock and a ppid getter. On main it fails with AssertionError: supervisor kept Electron running after its parent process went away: expected [] to include 'SIGTERM'.

• vp lint on the three touched files: clean.

• Runtime check of the real vp run dev:desktop tree on Linux (Xvfb, Electron 44, non-TTY, dash as /bin/sh):

  	Process tree	After kill -KILL <vp pack pid>
  main 8d3b061	vp pack --watch → /bin/sh -c node scripts/dev-electron.mjs → node scripts/dev-electron.mjs → electron	sh reparented to subreaper 1452. The supervisor stays alive, and touch dist-electron/main.cjs starts a new Electron (PID 710825 → 711522).
  this branch 85ebfe3	vp pack --watch → node scripts/dev-electron.mjs → electron	The supervisor and Electron are gone within 2 s, and the log shows before-quit received.

  On this branch, a rebuild (touch apps/desktop/src/main.ts) still replaces the supervisor through tsdown's SIGTERM, and the new supervisor is again a direct child of vp pack.

• Not verified: macOS and Windows. macOS /bin/sh already exec'd the last command, and exec keeps that. Windows keeps the previous command and has no parent check.

Built with Claude Opus 5.5 (implementation, tests) and Claude Sonnet 5.5 (review) via the Claude Code harness in T3 Code.

🤖 Generated with Claude Code

Independent proof audit

Verdict: PASS_WITH_NOTES. The matrix ran on main 8d3b061 and head 85ebfe3 on Linux (dash /bin/sh, plus bash in a private mount namespace). Exit codes come from strace.

Row	main	head
SIGKILL vp pack	Orphaned supervisor keeps relaunching Electron (18 relaunches), never exits	exit_group(129) at +1.5 s, session empty by +2 s
Same, after a rebuild	Bug reproduced	129 at +1.55 s
SIGTERM vp pack only	Supervisor orphaned, never exits	129 at +1.5 s (improvement)
SIGINT to the process group / SIGTERM to the session	130 / 143	130 / 143, same timing
Rebuild	New supervisor under a new sh -c	New supervisor is a direct child of vp pack
/bin/sh = bash	No sh, bug still reproduces	129 at +1.7 s / +2.1 s
unref timer	n/a	Does not keep the process alive (snippet exits in 0.04 s; control is held until timeout)
SIGKILL the grandparent	Survives	Survives (out of scope: only the direct parent is watched)

• Matrix: matrix.md. Summary: summary.md. Driver: run-row.sh, run-4b.sh.
• Key transcripts: main-r1, head-r1, head-r1 strace, main-r3, head-r3, head-r2b, head bash r1, unref + unit test. Audit rerun: transcript, strace.
• Notes: the supervisor exits 1.5–2.3 s after its parent dies, not always under 2 s. Before the 1 s check fires it can relaunch Electron once, so before-quit received does not appear in every run. The SIGINT row shows no before-quit on either tree.

Verification log

• Proof audit pass 1 (Claude Opus 5.5): GAPS against the original transcript (no exit code, no Ctrl-C/SIGTERM rows, no bash row, optimistic 2 s). All of these are covered by the capture above.
• Capture (Claude Opus 5.5): every row ran on both trees; main reproduces the bug, head passes every row.
• Proof audit pass 2 (Claude Opus 5.5): PASS_WITH_NOTES. Row by row against the files, SHAs match, head row 1 rerun independently (129 at +2.32 s).
• Second review (Claude Opus 5.5): VERDICT: MERGE. Low nits: the "without a shell in between" comment does not hold on the dev:electron task path, where the check is a no-op; the test does not assert exit code 129; the new it sits outside the describe block.
• Unverified: macOS and Windows (from code: macOS reparents to launchd, so the check fires; Windows is unchanged with no parent check).
• Evidence by Claude Opus 5.5 via the Claude Code harness in T3 Code.

[macroscopeapp]

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This is a focused desktop development-process fix that monitors the supervisor's parent and preserves the process relationship across the pack hook, with dedicated test coverage. Human review is required because the PR also adds a line-level lint-diagnostic suppression.

No code changes detected at 2df1d9e. Prior analysis still applies.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: fix/issue-19109
• File: .coderabbit.config.ts
• Commit: 2df1d9e
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: b7d5a802-c07e-49bc-94c0-663dd6d05a8c

Commits

Reviewing files that changed from the base of the PR and between 422d377 and 2df1d9e.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The desktop development launcher selects its post-pack command by platform. On non-Windows platforms, the supervisor checks its parent PID every second and initiates shutdown if it changes. Startup tests verify that the supervisor signals the Electron child after the parent PID changes.

Changes: Electron supervisor lifecycle

Layer / File(s)	Summary
Platform-specific launcher command apps/desktop/vite.config.ts	The post-pack command runs the Node script directly on Windows and uses exec on other platforms. The command is set as onSuccess only when desktop development is enabled.
Parent PID monitoring and shutdown apps/desktop/scripts/dev-electron.mjs, apps/desktop/scripts/electron-launcher.test.mjs	On non-Windows platforms, the supervisor checks whether its parent PID has changed and initiates shutdown with exit code 129 if it has. Tests use a fake clock to verify that the supervisor signals the Electron child after the parent PID changes.

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Parent
  participant Supervisor as dev-electron.mjs
  participant Child as Electron child
  Parent--xSupervisor: Parent process exits
  Supervisor->>Supervisor: Detect changed process.ppid
  Supervisor->>Child: Send SIGTERM
  Supervisor->>Supervisor: Exit with code 129

Loading

Suggested reviewers: maria-rcks, t3dotgg

Merge Risk: Minimal · previous review 422d3

The change makes the desktop development supervisor exit when its direct parent dies. No merge-blocking risk was found. macOS and Windows behavior was not exercised.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, uses the repository's conventional format, and clearly describes the primary change: stopping the desktop development supervisor when its parent process dies.
Description check		The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the failure, implementation, linked issue, test results, runtime evidence, limitations…
Linked Issues check		Active direct issue [#19109] requires the stale desktop development supervisor to stop when its parent disappears. dev-electron.mjs records the startup process.ppid, checks it every second on non-…
Out of Scope Changes check		The changes remain within [#19109]. The exec command removes the intermediate shell that could hide direct-parent death. The shared startup harness and fake clock support regression coverage for the…

Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the failure, implementation, linked issue, test results, runtime evidence, limitations, and agent usage. The approval evidence is summarized rather than quoting a specific maintainer comment, but the description is otherwise complete.

Full details: Linked Issues check

Explanation

Active direct issue [#19109] requires the stale desktop development supervisor to stop when its parent disappears. dev-electron.mjs records the startup process.ppid, checks it every second on non-Windows hosts, and calls shutdown(129) when the parent changes. vite.config.ts uses exec on non-Windows hosts so the supervisor directly observes the vp pack parent. The regression test verifies SIGTERM delivery and supervisor exit after parent reparenting. The changes do not need to implement stale-supervisor cleanup through cleanupStaleDevApps() or address [#19110].

Full details: Out of Scope Changes check

Explanation

The changes remain within [#19109]. The exec command removes the intermediate shell that could hide direct-parent death. The shared startup harness and fake clock support regression coverage for the parent-death behavior. The Windows command path preserves existing behavior because Windows does not reparent processes.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: fix/issue-19109
• File: .coderabbit.config.ts
• Commit: 422d377
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: c7314b10-7e86-4431-84b1-656b759611f2

Commits

Reviewing files that changed from the base of the PR and between 910d41e and 422d377.

Files selected for processing: 1

• apps/desktop/vite.config.ts

Limit details: You’ve used all 10 included reviews currently available.

[kvnloo]

Note

Written by claude-opus-5-5 (Claude Code), reviewed by gpt-6.1-sol (Hermes), on behalf of Kevin

Independent check at 85ebfe3 on Linux with native bash /bin/sh and repo-local vite-plus 1.3.0.

The direct-parent fix works in the original runtime checks: killing pack-bin.js --watch stopped the supervisor after about 2.5 seconds in both head runs; the base supervisor survived in both runs and relaunched Electron after touching main.cjs.

One process-name qualification for the repro: the pnpm-installed vp produced this tree:

node …/vite-plus/bin/vp pack --watch
└─ node …/vite-plus/dist/pack-bin.js --watch
   └─ node scripts/dev-electron.mjs
      └─ electron

The process to kill for the supported direct-parent row here is pack-bin.js, not the outer bin/vp wrapper. The original extra row reported that killing the wrapper leaves its child and supervisor running. That matches the PR's explicitly out-of-scope grandparent row, not a failure of the direct-parent guarantee.

Steps: use T3CODE_DESKTOP_DEV=1 node_modules/.bin/vp pack --watch in apps/desktop, a private HOME, a headless X server, and the required server bundle/dev HTTP endpoint. Record the parent PID before sending SIGKILL.

Not checked: full vp run dev:desktop topology, macOS, Windows, global native vp.

[juliusmarminge]

Evidence

Ran the real Electron desktop on Linux/Xvfb at 1920×1080 with fresh isolated homes and seeding disabled. Recorded the supervisor's direct parent, killed only that PID with SIGKILL, waited 8 seconds, then touched apps/desktop/dist-electron/main.cjs. Repeated after a source rebuild created a replacement supervisor. The footer shows live PID observations; only the setup machine label was redacted.

Before	After
3a8c250cb0	910d41eb6e
Initial supervisor recording: orphan supervisor survives and Electron reopens.	Initial supervisor recording: Electron closes, supervisor exits, and the file touch causes no relaunch.
After rebuild recording: replacement supervisor survives and reopens Electron.	After rebuild recording: replacement supervisor exits and Electron stays closed.

Reproduction driver · Process transcript · Video redaction script

AI-assisted; verified with the tests and tools listed above.

Not shown:

• macOS parent-death behavior was not exercised; this comparison used Linux.

[juliusmarminge]

The automated pipeline stopped on this pull request and will not merge it:

someone else pushed to fix/issue-19109 (head is now 422d377); the pipeline does not build on commits it did not review

It stays open for a person to finish or close.