fix(windows): close Job teardown review gaps

This commit is contained in:
OrcaWin
2026-07-20 04:44:31 -07:00
parent 3acb1a7ccf
commit 424a04a30e
7 changed files with 374 additions and 57 deletions
+185 -23
View File
@@ -21,7 +21,7 @@ index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..6047590bc9558dabacd3ece5114ed41c
},
'targets' : [
diff --git a/lib/conpty_console_list_agent.js b/lib/conpty_console_list_agent.js
index 8c4fca9022a6d6f015bca87f61625cde2278f428..0a01730616488119aa21ef441cf3c441e02a974c 100644
index 8c4fca9022a6d6f015bca87f61625cde2278f428..0b71eba1d0fdb312bd46e5f47c218bd5330f023a 100644
--- a/lib/conpty_console_list_agent.js
+++ b/lib/conpty_console_list_agent.js
@@ -10,7 +10,14 @@ Object.defineProperty(exports, "__esModule", { value: true });
@@ -42,7 +42,7 @@ index 8c4fca9022a6d6f015bca87f61625cde2278f428..0a01730616488119aa21ef441cf3c441
//# sourceMappingURL=conpty_console_list_agent.js.map
\ No newline at end of file
diff --git a/lib/unixTerminal.js b/lib/unixTerminal.js
index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088bf1865877 100644
index 1ec12f796a822c78fba9ad7f6448c3987e325c23..24cbb71bce12f8e6ec617918df1e16bafd6adff1 100644
--- a/lib/unixTerminal.js
+++ b/lib/unixTerminal.js
@@ -28,8 +28,12 @@ var native = utils_1.loadNativeModule('pty');
@@ -61,10 +61,10 @@ index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088b
var DEFAULT_NAME = 'xterm';
var DESTROY_SOCKET_TIMEOUT_MS = 200;
diff --git a/lib/windowsPtyAgent.js b/lib/windowsPtyAgent.js
index a358ffb177357e177661033c1b092f9c9d0e5f5a..b34cfef73fcdcf7897b6f4646ec464f348311330 100644
index a358ffb177357e177661033c1b092f9c9d0e5f5a..187d72f22bb9ee8dbf444465d0febb302be4aa54 100644
--- a/lib/windowsPtyAgent.js
+++ b/lib/windowsPtyAgent.js
@@ -26,14 +26,17 @@ var FLUSH_DATA_INTERVAL = 1000;
@@ -26,17 +26,23 @@ var FLUSH_DATA_INTERVAL = 1000;
* and winpty.
*/
var WindowsPtyAgent = /** @class */ (function () {
@@ -79,29 +79,50 @@ index a358ffb177357e177661033c1b092f9c9d0e5f5a..b34cfef73fcdcf7897b6f4646ec464f3
this._pid = 0;
this._innerPid = 0;
+ this._processExited = false;
+ this._windowsAgentJob = false;
+ this._processExitListeners = [];
if (this._useConpty === undefined || this._useConpty === true) {
this._useConpty = this._getWindowsBuildNumber() >= 18309;
}
@@ -86,7 +89,7 @@ var WindowsPtyAgent = /** @class */ (function () {
+ // Why: a requested Job is effective only when runtime detection actually selects ConPTY.
+ this._windowsAgentJob = Boolean(this._useConpty && windowsAgentJob);
if (this._useConpty) {
if (!conptyNative) {
conptyNative = utils_1.loadNativeModule('conpty').module;
@@ -86,7 +92,7 @@ var WindowsPtyAgent = /** @class */ (function () {
});
this._inSocket.setEncoding('utf8');
if (this._useConpty) {
- var connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, function (c) { return _this._$onProcessExit(c); });
+ var connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, windowsAgentJob, function (c) { return _this._$onProcessExit(c); });
+ var connect = this._ptyNative.connect(this._pty, commandLine, cwd, env, this._useConptyDll, this._windowsAgentJob, function (c) { return _this._$onProcessExit(c); });
this._innerPid = connect.pid;
}
}
@@ -130,6 +133,20 @@ var WindowsPtyAgent = /** @class */ (function () {
@@ -115,6 +121,11 @@ var WindowsPtyAgent = /** @class */ (function () {
enumerable: false,
configurable: true
});
+ Object.defineProperty(WindowsPtyAgent.prototype, "windowsAgentJob", {
+ get: function () { return this._windowsAgentJob; },
+ enumerable: false,
+ configurable: true
+ });
WindowsPtyAgent.prototype.resize = function (cols, rows) {
if (this._useConpty) {
if (this._exitCode !== undefined) {
@@ -130,6 +141,23 @@ var WindowsPtyAgent = /** @class */ (function () {
this._ptyNative.clear(this._pty, this._useConptyDll);
}
};
+ WindowsPtyAgent.prototype.terminateJobTree = function (timeoutMs) {
+ if (!this._useConpty) {
+ var _a, _b;
+ if (!this._windowsAgentJob) {
+ return undefined;
+ }
+ var _a;
+ return (_a = this._ptyNative.terminateJobTree) === null || _a === void 0 ? void 0 : _a.call(this._ptyNative, this._pty, timeoutMs);
+ if (this._processExited) {
+ return Promise.resolve(true);
+ }
+ return (_b = (_a = this._ptyNative).terminateJobTree) === null || _b === void 0 ? void 0 : _b.call(_a, this._pty, timeoutMs);
+ };
+ WindowsPtyAgent.prototype.onProcessExit = function (listener) {
+ if (this._processExited) {
@@ -113,7 +134,7 @@ index a358ffb177357e177661033c1b092f9c9d0e5f5a..b34cfef73fcdcf7897b6f4646ec464f3
WindowsPtyAgent.prototype.kill = function () {
var _this = this;
// Tell the agent to kill the pty, this releases handles to the process
@@ -221,6 +238,10 @@ var WindowsPtyAgent = /** @class */ (function () {
@@ -221,6 +249,10 @@ var WindowsPtyAgent = /** @class */ (function () {
WindowsPtyAgent.prototype._$onProcessExit = function (exitCode) {
var _this = this;
this._exitCode = exitCode;
@@ -124,8 +145,39 @@ index a358ffb177357e177661033c1b092f9c9d0e5f5a..b34cfef73fcdcf7897b6f4646ec464f3
if (!this._useConptyDll) {
this._flushDataAndCleanUp();
this._outSocket.on('data', function () { return _this._flushDataAndCleanUp(); });
diff --git a/lib/windowsPtyAgent.test.js b/lib/windowsPtyAgent.test.js
index 15bbf5ba397ab02a5ec6a11b1c0c6f6ffd137be0..d9a0cf1af2da1cdd56ff6edae0bfa41580b51c60 100644
--- a/lib/windowsPtyAgent.test.js
+++ b/lib/windowsPtyAgent.test.js
@@ -10,6 +10,26 @@ function check(file, args, expected) {
assert.equal(windowsPtyAgent_1.argsToCommandLine(file, args), expected);
}
if (process.platform === 'win32') {
+ describe('Windows agent Job control', function () {
+ it('reports an already-exited owned Job as drained without another native lookup', function () {
+ var terminateJobTree = function () { return Promise.resolve(false); };
+ var agent = Object.create(windowsPtyAgent_1.WindowsPtyAgent.prototype);
+ agent._windowsAgentJob = true;
+ agent._processExited = true;
+ agent._pty = 1;
+ agent._ptyNative = { terminateJobTree: terminateJobTree };
+ assert.strictEqual(agent.windowsAgentJob, true);
+ return agent.terminateJobTree(1000).then(function (drained) {
+ assert.strictEqual(drained, true);
+ });
+ });
+ it('does not expose teardown when Job ownership was not established', function () {
+ var agent = Object.create(windowsPtyAgent_1.WindowsPtyAgent.prototype);
+ agent._windowsAgentJob = false;
+ assert.strictEqual(agent.windowsAgentJob, false);
+ assert.strictEqual(agent.terminateJobTree(1000), undefined);
+ });
+ });
describe('argsToCommandLine', function () {
describe('Plain strings', function () {
it('doesn\'t quote plain string', function () {
diff --git a/lib/windowsTerminal.js b/lib/windowsTerminal.js
index 3c38f89d14cece8081782a9600ebc582b7c2a62a..7a488cbd891e0227b367857af894b4818a7a6031 100644
index 3c38f89d14cece8081782a9600ebc582b7c2a62a..52e47c98194daf6f64b22fcaa647077acf989371 100644
--- a/lib/windowsTerminal.js
+++ b/lib/windowsTerminal.js
@@ -48,8 +48,23 @@ var WindowsTerminal = /** @class */ (function (_super) {
@@ -139,7 +191,7 @@ index 3c38f89d14cece8081782a9600ebc582b7c2a62a..7a488cbd891e0227b367857af894b481
+ var agentProcessExited = false;
+ var exitEmitted = false;
+ var emitExitWhenSafe = function () {
+ if (exitEmitted || !socketClosed || (opt.windowsAgentJob && !agentProcessExited)) {
+ if (exitEmitted || !socketClosed || (_this._agent.windowsAgentJob && !agentProcessExited)) {
+ return;
+ }
+ exitEmitted = true;
@@ -176,6 +228,30 @@ index 3c38f89d14cece8081782a9600ebc582b7c2a62a..7a488cbd891e0227b367857af894b481
WindowsTerminal.prototype.kill = function (signal) {
var _this = this;
this._deferNoArgs(function () {
diff --git a/lib/windowsTerminal.test.js b/lib/windowsTerminal.test.js
index af5f343d17f012f330fe2292083f297768ff3b5e..6b558c789fc77b58c20cd9564d03e93e54164f54 100644
--- a/lib/windowsTerminal.test.js
+++ b/lib/windowsTerminal.test.js
@@ -88,6 +88,19 @@ function pollForProcessTreeSize(pid, size, intervalMs, timeoutMs) {
});
}
if (process.platform === 'win32') {
+ describe('WindowsTerminal Job capability fallback', function () {
+ it('does not wait for a Job callback when a requested Job falls back to winpty', function (done) {
+ this.timeout(10000);
+ var term = new windowsTerminal_1.WindowsTerminal('cmd.exe', '/C exit', {
+ useConpty: false,
+ windowsAgentJob: true
+ });
+ term.on('exit', function (code) {
+ assert.strictEqual(code, 0);
+ done();
+ });
+ });
+ });
[[false, false], [true, false], [true, true]].forEach(function (_a) {
var useConpty = _a[0], useConptyDll = _a[1];
describe("WindowsTerminal (useConpty = " + useConpty + ", useConptyDll = " + useConptyDll + ")", function () {
diff --git a/src/conpty_console_list_agent.ts b/src/conpty_console_list_agent.ts
index 181ccabbbe9c4948a9725fb1db907a68e9de01fc..67f31facf85562b67adbfbd04ce28ddd8eeb4a79 100644
--- a/src/conpty_console_list_agent.ts
@@ -774,20 +850,72 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..16f8d99c627159a35cb643a2752ded30
return exports;
};
diff --git a/src/windowsPtyAgent.test.ts b/src/windowsPtyAgent.test.ts
index dc2104b34fd396722a9e30821ff0b7dea18b4295..23510c3d7c85ecc911b94719832abe5f2b90c1e3 100644
--- a/src/windowsPtyAgent.test.ts
+++ b/src/windowsPtyAgent.test.ts
@@ -4,13 +4,37 @@
*/
import * as assert from 'assert';
-import { argsToCommandLine } from './windowsPtyAgent';
+import { argsToCommandLine, WindowsPtyAgent } from './windowsPtyAgent';
function check(file: string, args: string | string[], expected: string): void {
assert.equal(argsToCommandLine(file, args), expected);
}
if (process.platform === 'win32') {
+ describe('Windows agent Job control', () => {
+ it('reports an already-exited owned Job as drained without another native lookup', () => {
+ const terminateJobTree = () => Promise.resolve(false);
+ const agent = Object.create(WindowsPtyAgent.prototype) as any;
+ agent._windowsAgentJob = true;
+ agent._processExited = true;
+ agent._pty = 1;
+ agent._ptyNative = { terminateJobTree };
+
+ assert.strictEqual(agent.windowsAgentJob, true);
+ return agent.terminateJobTree(1000).then((drained: boolean) => {
+ assert.strictEqual(drained, true);
+ });
+ });
+
+ it('does not expose teardown when Job ownership was not established', () => {
+ const agent = Object.create(WindowsPtyAgent.prototype) as any;
+ agent._windowsAgentJob = false;
+
+ assert.strictEqual(agent.windowsAgentJob, false);
+ assert.strictEqual(agent.terminateJobTree(1000), undefined);
+ });
+ });
+
describe('argsToCommandLine', () => {
describe('Plain strings', () => {
it('doesn\'t quote plain string', () => {
diff --git a/src/windowsPtyAgent.ts b/src/windowsPtyAgent.ts
index d7054449516f0c9a62af351c2caa17331206d530..5cc9914ee1f1b8b40dadfb8164763357f5549300 100644
index d7054449516f0c9a62af351c2caa17331206d530..5cc174373171329f36860103edab95323da89ed7 100644
--- a/src/windowsPtyAgent.ts
+++ b/src/windowsPtyAgent.ts
@@ -34,6 +34,8 @@ export class WindowsPtyAgent {
@@ -34,6 +34,9 @@ export class WindowsPtyAgent {
private _innerPid: number = 0;
private _closeTimeout: NodeJS.Timer | undefined;
private _exitCode: number | undefined;
+ private _processExited: boolean = false;
+ private _windowsAgentJob: boolean = false;
+ private _processExitListeners: (() => void)[] = [];
private _conoutSocketWorker: ConoutConnection;
private _fd: any;
@@ -56,7 +58,8 @@ export class WindowsPtyAgent {
@@ -45,6 +48,7 @@ export class WindowsPtyAgent {
public get fd(): any { return this._fd; }
public get innerPid(): number { return this._innerPid; }
public get pty(): number { return this._pty; }
+ public get windowsAgentJob(): boolean { return this._windowsAgentJob; }
constructor(
file: string,
@@ -56,11 +60,14 @@ export class WindowsPtyAgent {
debug: boolean,
private _useConpty: boolean | undefined,
private _useConptyDll: boolean = false,
@@ -797,23 +925,32 @@ index d7054449516f0c9a62af351c2caa17331206d530..5cc9914ee1f1b8b40dadfb8164763357
) {
if (this._useConpty === undefined || this._useConpty === true) {
this._useConpty = this._getWindowsBuildNumber() >= 18309;
@@ -116,7 +119,7 @@ export class WindowsPtyAgent {
}
+ // Why: a requested Job is effective only when runtime detection actually selects ConPTY.
+ this._windowsAgentJob = Boolean(this._useConpty && windowsAgentJob);
if (this._useConpty) {
if (!conptyNative) {
conptyNative = loadNativeModule('conpty').module;
@@ -116,7 +123,7 @@ export class WindowsPtyAgent {
this._inSocket.setEncoding('utf8');
if (this._useConpty) {
- const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._useConptyDll, c => this._$onProcessExit(c));
+ const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._useConptyDll, windowsAgentJob, c => this._$onProcessExit(c));
+ const connect = (this._ptyNative as IConptyNative).connect(this._pty, commandLine, cwd, env, this._useConptyDll, this._windowsAgentJob, c => this._$onProcessExit(c));
this._innerPid = connect.pid;
}
}
@@ -138,6 +141,21 @@ export class WindowsPtyAgent {
@@ -138,6 +145,24 @@ export class WindowsPtyAgent {
}
}
+ public terminateJobTree(timeoutMs: number): Promise<boolean> | undefined {
+ if (!this._useConpty) {
+ if (!this._windowsAgentJob) {
+ return undefined;
+ }
+ if (this._processExited) {
+ return Promise.resolve(true);
+ }
+ return (this._ptyNative as IConptyNative).terminateJobTree?.(this._pty, timeoutMs);
+ }
+
@@ -828,7 +965,7 @@ index d7054449516f0c9a62af351c2caa17331206d530..5cc9914ee1f1b8b40dadfb8164763357
public kill(): void {
// Tell the agent to kill the pty, this releases handles to the process
if (this._useConpty) {
@@ -223,6 +241,10 @@ export class WindowsPtyAgent {
@@ -223,6 +248,10 @@ export class WindowsPtyAgent {
*/
private _$onProcessExit(exitCode: number): void {
this._exitCode = exitCode;
@@ -839,8 +976,33 @@ index d7054449516f0c9a62af351c2caa17331206d530..5cc9914ee1f1b8b40dadfb8164763357
if (!this._useConptyDll) {
this._flushDataAndCleanUp();
this._outSocket.on('data', () => this._flushDataAndCleanUp());
diff --git a/src/windowsTerminal.test.ts b/src/windowsTerminal.test.ts
index 8f1274edf6dcf6e9257ba9d711099791391ec778..afc5813d26ce0027b826d326fffdbc3c242753aa 100644
--- a/src/windowsTerminal.test.ts
+++ b/src/windowsTerminal.test.ts
@@ -92,6 +92,20 @@ function pollForProcessTreeSize(pid: number, size: number, intervalMs: number =
}
if (process.platform === 'win32') {
+ describe('WindowsTerminal Job capability fallback', () => {
+ it('does not wait for a Job callback when a requested Job falls back to winpty', function(done) {
+ this.timeout(10000);
+ const term = new WindowsTerminal('cmd.exe', '/C exit', {
+ useConpty: false,
+ windowsAgentJob: true
+ });
+ term.on('exit', code => {
+ assert.strictEqual(code, 0);
+ done();
+ });
+ });
+ });
+
[[false, false], [true, false], [true, true]].forEach(([useConpty, useConptyDll]) => {
describe(`WindowsTerminal (useConpty = ${useConpty}, useConptyDll = ${useConptyDll})`, () => {
describe('kill', () => {
diff --git a/src/windowsTerminal.ts b/src/windowsTerminal.ts
index 13f6c6dbb3d7965c4699568f4704be3b6a33d947..23f2f2d5b7f95df60a22adad9c4d7cbdc1a3dd5f 100644
index 13f6c6dbb3d7965c4699568f4704be3b6a33d947..db6ddd94a35b034c0fb998003e4332c28512809d 100644
--- a/src/windowsTerminal.ts
+++ b/src/windowsTerminal.ts
@@ -48,8 +48,23 @@ export class WindowsTerminal extends Terminal {
@@ -854,7 +1016,7 @@ index 13f6c6dbb3d7965c4699568f4704be3b6a33d947..23f2f2d5b7f95df60a22adad9c4d7cbd
+ let agentProcessExited = false;
+ let exitEmitted = false;
+ const emitExitWhenSafe = (): void => {
+ if (exitEmitted || !socketClosed || (opt.windowsAgentJob && !agentProcessExited)) {
+ if (exitEmitted || !socketClosed || (this._agent.windowsAgentJob && !agentProcessExited)) {
+ return;
+ }
+ exitEmitted = true;
+14 -6
View File
@@ -712,17 +712,17 @@
"ssh",
"runtime"
],
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Windows 2022 Actions red/green runs prove production store.removeWorktree(id, true) retires an agent PTY and its reparented native lock-holder before deleting the worktree, plain terminal close preserves an intentionally detached child, and natural agent-root exit publishes PTY exit only after its detached Job child is absent. Deterministic tests cover native Windows agent-only Job selection, fail-closed Job teardown and bridge errors, safe failure when Job ownership is unavailable, adopted-v24 destructive-shutdown refusal, daemon WSL exclusion, daemon-wide shutdown, daemon and SSH routing, local/daemon pending-snapshot ownership across natural exit, stale-root descendant-signal suppression, graceful-to-immediate kill upgrades, duplicate-kill completion sharing, locale-stable bounded/fresh/coalesced process-table reads, deadline-safe successor scans, cycle-safe linear descendant traversal, target-only escalation indexing, 32-wide bulk teardown, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, WSL, SSH, and remote-runtime process evidence remains pending.",
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Windows 2022 Actions red/green runs prove production store.removeWorktree(id, true) retires an agent PTY and its reparented native lock-holder before deleting the worktree, plain terminal close preserves an intentionally detached child, and natural agent-root exit publishes PTY exit only after its detached Job child is absent. Deterministic tests cover native Windows agent-only Job selection, fail-closed Job teardown and bridge errors, safe failure when Job ownership is unavailable, adopted-v24 destructive-shutdown refusal, recovered legacy routing and duplicate daemon-session ownership refusal, daemon WSL exclusion, daemon-wide shutdown, daemon and SSH routing, local/daemon pending-snapshot ownership across natural exit, stale-root descendant-signal suppression, graceful-to-immediate kill upgrades, duplicate-kill completion sharing, locale-stable bounded/fresh/coalesced process-table reads, deadline-safe successor scans, cycle-safe linear descendant traversal, target-only escalation indexing, 32-wide bulk teardown, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, WSL, SSH, and remote-runtime process evidence remains pending.",
"motivatingLinks": [
"https://github.com/stablyai/orca/pull/8628",
"https://github.com/stablyai/orca/pull/8706",
"https://github.com/stablyai/orca/issues/9045"
],
"invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a native Windows agent PTY owns its spawn-time process family in a kill-on-close Job that must drain before destructive worktree deletion succeeds, while plain terminals, WSL, and SSH retain their existing semantics; a terminating id remains reserved through natural exit, duplicate callers await the same completion, and immediate teardown upgrades any graceful request without signalling a recycled PID or a descendant tree after root ownership is lost; process-table work is locale-stable, bounded, fresh for each post-start request, same-turn coalesced, and begins within the requesting caller's deadline, including bulk worktree cleanup; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.",
"oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward. On Windows, prove the fixture is Git-registered, launch PowerShell shell -> agent Node process -> exited launcher -> reparented native lock-holder, invoke production store.removeWorktree(id, true), and prove the exact PTY, root, agent, lock-holder, worktree directory, Git registration, and worktree list entry are absent; separately prove a plain terminal's intentionally detached child survives close. A natural-root-exit journey records the public PTY exit callback and requires the detached Job child to be absent at that exact callback. Unit tests require agent-only spawn-time Job selection, Job termination success before root close, fail-closed timeout/rejection/synchronous-throw behavior, adopted-v24 destructive-shutdown refusal, daemon WSL exclusion, safe failure without PID-only fallback when Job ownership is unavailable, daemon-wide routing through the same teardown, naturally exited id reservation, bounded locale-stable process-table reads, duplicate completion sharing, canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.",
"oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward. On Windows, prove the fixture is Git-registered, launch PowerShell shell -> agent Node process -> exited launcher -> reparented native lock-holder, invoke production store.removeWorktree(id, true), and prove the exact PTY, root, agent, lock-holder, worktree directory, Git registration, and worktree list entry are absent; separately prove a plain terminal's intentionally detached child survives close. A natural-root-exit journey records the public PTY exit callback and requires the detached Job child to be absent at that exact callback. Unit tests require agent-only spawn-time Job selection, Job termination success before root close, fail-closed timeout/rejection/synchronous-throw behavior, adopted-v24 destructive-shutdown refusal, recovered legacy route refresh, duplicate cross-daemon ownership refusal, daemon WSL exclusion, safe failure without PID-only fallback when Job ownership is unavailable, daemon-wide routing through the same teardown, naturally exited id reservation, bounded locale-stable process-table reads, duplicate completion sharing, canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.",
"commands": [
"pnpm dlx node@24 ./node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/agent-hooks/server-pane-authority.test.ts src/main/ipc/agent-hooks.test.ts src/main/ipc/agent-pane-authority-ownership.test.ts src/main/ipc/pty-management.test.ts src/main/persistence.test.ts src/renderer/src/store/slices/agent-pane-authority.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/renderer/src/store/slices/terminal-tab-retirement.test.ts src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts",
"pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-session-teardown.test.ts src/main/daemon/pty-subprocess.test.ts src/main/providers/local-pty-provider.test.ts src/main/runtime/worktree-teardown.test.ts",
"pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-session-teardown.test.ts src/main/daemon/pty-subprocess.test.ts src/main/providers/local-pty-provider.test.ts src/main/runtime/worktree-teardown.test.ts",
"pnpm run test:e2e -- tests/e2e/terminal-parked-close-retirement.spec.ts --workers=1",
"pnpm run test:e2e -- tests/e2e/agent-descendant-process-kill.spec.ts --workers=1",
"pnpm run test:e2e -- tests/e2e/windows-agent-job-natural-exit.spec.ts --workers=1",
@@ -736,6 +736,7 @@
"src/main/persistence.test.ts",
"src/main/pty-descendant-termination.test.ts",
"src/main/daemon/daemon-pty-adapter.test.ts",
"src/main/daemon/daemon-pty-router.test.ts",
"src/main/daemon/session.test.ts",
"src/main/daemon/terminal-host.test.ts",
"src/main/daemon/terminal-session-teardown.test.ts",
@@ -802,7 +803,14 @@
{
"file": "src/main/daemon/daemon-pty-adapter.test.ts",
"assertions": [
"an adopted Windows v24 daemon rejects destructive shutdown before connecting or sending kill while ordinary close remains compatible"
"an adopted Windows v24 daemon rejects deadline-bearing agent, plain, and WSL shutdown before connecting or sending kill while ordinary close remains compatible"
]
},
{
"file": "src/main/daemon/daemon-pty-router.test.ts",
"assertions": [
"a successful list refresh restores recovered legacy routing and rejects duplicate cross-daemon session ownership before destructive teardown",
"one exit event cannot clear ambiguity while two other daemon owners remain"
]
},
{
@@ -897,7 +905,7 @@
},
"redGreenEvidence": {
"status": "complete",
"evidence": "The Windows issue #9045 test failed at test-only e5a67d121 in Actions run https://github.com/stablyai/orca/actions/runs/29731005277: production worktree removal returned 'Failed to physically stop every PTY', while the root, agent, and reparented lock-holder remained alive. The strengthened test passed at implementation ecb8652d6 in run https://github.com/stablyai/orca/actions/runs/29733816749, proving registration before removal, removal ok, and all recorded processes, the worktree directory, Git registration, and list entry absent. Final 288116dfd repeated that production proof in https://github.com/stablyai/orca/actions/runs/29736500566 and also proved natural public exit waits for detached Job-child absence."
"evidence": "The Windows issue #9045 test failed at test-only e5a67d121 in Actions run https://github.com/stablyai/orca/actions/runs/29731005277: the root, agent, and reparented lock-holder were proved alive immediately before production worktree removal returned 'Failed to physically stop every PTY'. The strengthened test passed at implementation ecb8652d6 in run https://github.com/stablyai/orca/actions/runs/29733816749, proving registration before removal, removal ok, and all recorded processes, the worktree directory, Git registration, and list entry absent. Final 288116dfd repeated that production proof in https://github.com/stablyai/orca/actions/runs/29736500566 and also proved natural public exit waits for detached Job-child absence."
},
"performanceBudget": {
"required": true,
@@ -912,7 +920,7 @@
"Live process proof covers macOS local PTYs and native Windows agent/plain PTYs, but not Linux, WSL, SSH, or remote-runtime PTYs.",
"Disconnected SSH relay death still requires reconnect-aware provider ownership.",
"Daemon owner leases and durable retry inventory remain follow-up hardening.",
"Existing adoptable daemon protocol v24 sessions predate spawn-time Windows Job ownership, so users must start a fresh v25 agent session to gain this protection.",
"Existing adopted Windows v24 sessions (agent, plain, or WSL) lack spawn-time Job capability metadata, so deadline-bearing worktree deletion fails closed until those sessions are closed or restarted under v25; fresh v25 plain and WSL behavior is unchanged.",
"SSH-hosted, WSL, and remote-runtime PTYs retain their host-specific teardown paths and are not placed in the local Windows Job.",
"A process born in the capture second is SIGTERMed but not SIGKILLed because ps cannot prove its recycled-PID identity.",
"A descendant orphaned before or during root ownership loss requires the separate crash-orphan sweep and is not recovered from a stale kill-time snapshot."
+3 -3
View File
@@ -21,7 +21,7 @@ patchedDependencies:
hash: 9c1de9931d86864923ff53bc9d64474a86478085ac81ea4e432648c7e23d702c
path: config/patches/@xterm__xterm@6.1.0-beta.287.patch
node-pty@1.1.0:
hash: 942526bf93620d3912de272d03406fdce9356755039f9fefe6265012f1806e8a
hash: 192488cceb7c9105c5a76a15da8203d297d81b1b73b826430bc852ae348a957e
path: config/patches/node-pty@1.1.0.patch
importers:
@@ -63,7 +63,7 @@ importers:
version: 3.3.1
node-pty:
specifier: ^1.1.0
version: 1.1.0(patch_hash=942526bf93620d3912de272d03406fdce9356755039f9fefe6265012f1806e8a)
version: 1.1.0(patch_hash=192488cceb7c9105c5a76a15da8203d297d81b1b73b826430bc852ae348a957e)
posthog-node:
specifier: ^5.33.3
version: 5.33.3
@@ -12043,7 +12043,7 @@ snapshots:
undici: 6.27.0
which: 6.0.1
node-pty@1.1.0(patch_hash=942526bf93620d3912de272d03406fdce9356755039f9fefe6265012f1806e8a):
node-pty@1.1.0(patch_hash=192488cceb7c9105c5a76a15da8203d297d81b1b73b826430bc852ae348a957e):
dependencies:
node-addon-api: 7.1.1
+13 -11
View File
@@ -532,7 +532,7 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
})
describe('shutdown', () => {
it('fails closed for destructive shutdown routed to an adopted Windows v24 daemon', async () => {
it('blocks every adopted Windows v24 session kind from deadline teardown but keeps ordinary close', async () => {
const ensureConnectedSpy = vi
.spyOn(DaemonClient.prototype, 'ensureConnected')
.mockResolvedValue()
@@ -542,21 +542,23 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
const legacy = new DaemonPtyAdapter({ socketPath, tokenPath, protocolVersion: 24 })
try {
await expect(
legacy.shutdown('legacy-session', {
immediate: true,
deadlineMs: Date.now() + 5_000
})
).rejects.toThrow('requires daemon protocol 25 or newer')
// Why: v24 did not record whether a session was agent, plain, or WSL,
// so migration safety must reject deadline teardown for all three.
for (const id of ['legacy-agent', 'legacy-plain', 'legacy-wsl']) {
await expect(
legacy.shutdown(id, {
immediate: true,
deadlineMs: Date.now() + 5_000
})
).rejects.toThrow('requires daemon protocol 25 or newer')
}
expect(ensureConnectedSpy).not.toHaveBeenCalled()
expect(requestSpy).not.toHaveBeenCalled()
await expect(
legacy.shutdown('legacy-session', { immediate: true })
).resolves.toBeUndefined()
await expect(legacy.shutdown('legacy-plain', { immediate: true })).resolves.toBeUndefined()
expect(requestSpy).toHaveBeenCalledWith(
'kill',
{ sessionId: 'legacy-session', immediate: true },
{ sessionId: 'legacy-plain', immediate: true },
undefined
)
} finally {
+54
View File
@@ -274,6 +274,60 @@ describe('DaemonPtyRouter', () => {
await expect(router.listProcesses()).rejects.toThrow('legacy unavailable')
})
it('refreshes legacy routing after a successful listProcesses recovery', async () => {
const current = createAdapter('current', ['current-session'])
const legacy = createAdapter('legacy', ['legacy-session'])
vi.mocked(legacy.listProcesses).mockRejectedValueOnce(new Error('legacy unavailable'))
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
await router.discoverLegacySessions()
router.write('legacy-session', 'before recovery\n')
expect(current.write).toHaveBeenCalledWith('legacy-session', 'before recovery\n')
await expect(router.listProcesses()).resolves.toHaveLength(2)
router.write('legacy-session', 'after recovery\n')
expect(legacy.write).toHaveBeenCalledWith('legacy-session', 'after recovery\n')
})
it('rejects destructive teardown when recovered daemons report a duplicate session id', async () => {
const current = createAdapter('current', ['duplicate-session'])
const legacy = createAdapter('legacy', ['duplicate-session'])
vi.mocked(legacy.listProcesses).mockRejectedValueOnce(new Error('legacy unavailable'))
const router = new DaemonPtyRouter({ current, legacy: [legacy] })
await router.discoverLegacySessions()
await expect(router.listProcesses()).rejects.toThrow(
'Ambiguous PTY session ownership across daemons: duplicate-session'
)
await expect(
router.shutdown('duplicate-session', {
immediate: true,
deadlineMs: Date.now() + 5_000
})
).rejects.toThrow('Ambiguous PTY session ownership')
expect(current.shutdown).not.toHaveBeenCalled()
expect(legacy.shutdown).not.toHaveBeenCalled()
})
it('keeps duplicate ownership ambiguous when one of three owners exits', async () => {
const current = createAdapter('current', ['duplicate-session'])
const firstLegacy = createAdapter('legacy-a', ['duplicate-session'])
const secondLegacy = createAdapter('legacy-b', ['duplicate-session'])
const router = new DaemonPtyRouter({ current, legacy: [firstLegacy, secondLegacy] })
await expect(router.listProcesses()).rejects.toThrow('Ambiguous PTY session ownership')
current.emitExit('duplicate-session', 0)
await expect(
router.shutdown('duplicate-session', {
immediate: true,
deadlineMs: Date.now() + 5_000
})
).rejects.toThrow('Ambiguous PTY session ownership')
expect(firstLegacy.shutdown).not.toHaveBeenCalled()
expect(secondLegacy.shutdown).not.toHaveBeenCalled()
})
it('merges startup reconciliation and updates route mappings', async () => {
const current = createAdapter('current', [], {
alive: ['current-alive'],
+26 -14
View File
@@ -1,4 +1,5 @@
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
import { DaemonPtySessionRouting } from './daemon-pty-session-routing'
import type {
IPtyProvider,
PtyBackgroundStreamEvent,
@@ -11,7 +12,7 @@ import type {
export class DaemonPtyRouter implements IPtyProvider {
private current: DaemonPtyAdapter
private legacy: DaemonPtyAdapter[]
private sessionAdapters = new Map<string, DaemonPtyAdapter>()
private sessionRouting = new DaemonPtySessionRouting()
private unsubscribers: (() => void)[] = []
private dataListeners: ((payload: {
id: string
@@ -34,7 +35,7 @@ export class DaemonPtyRouter implements IPtyProvider {
}
}),
adapter.onExit((payload) => {
this.sessionAdapters.delete(payload.id)
this.sessionRouting.remove(payload.id, adapter)
for (const listener of this.exitListeners) {
listener(payload)
}
@@ -48,7 +49,7 @@ export class DaemonPtyRouter implements IPtyProvider {
try {
const sessions = await adapter.listProcesses()
for (const session of sessions) {
this.sessionAdapters.set(session.id, adapter)
this.sessionRouting.add(session.id, adapter)
}
} catch (error) {
console.warn('[daemon] Failed to discover legacy daemon sessions', error)
@@ -57,10 +58,10 @@ export class DaemonPtyRouter implements IPtyProvider {
}
async spawn(opts: PtySpawnOptions): Promise<PtySpawnResult> {
const adapter = opts.sessionId ? this.sessionAdapters.get(opts.sessionId) : undefined
const adapter = opts.sessionId ? this.sessionRouting.get(opts.sessionId) : undefined
const target = adapter ?? this.current
const result = await target.spawn(opts)
this.sessionAdapters.set(result.id, target)
this.sessionRouting.add(result.id, target)
return result
}
@@ -74,7 +75,7 @@ export class DaemonPtyRouter implements IPtyProvider {
}
hasPty(id: string): boolean {
const routed = this.sessionAdapters.get(id)
const routed = this.sessionRouting.get(id)
if (routed) {
return routed.hasPty(id)
}
@@ -105,7 +106,11 @@ export class DaemonPtyRouter implements IPtyProvider {
id: string,
opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number }
): Promise<void> {
await this.adapterFor(id).shutdown(id, opts)
if (opts.deadlineMs !== undefined && this.sessionRouting.isAmbiguous(id)) {
throw this.ambiguousOwnershipError([id])
}
const adapter = this.adapterFor(id)
await adapter.shutdown(id, opts)
// Why: sleep passes keepHistory=true and re-spawns against the same
// sessionId on wake. If we delete the routing entry here, adapterFor()
// falls back to `this.current` on wake — for a session that originally
@@ -113,7 +118,7 @@ export class DaemonPtyRouter implements IPtyProvider {
// createOrAttach lands on the wrong adapter and creates a fresh session,
// losing the cold-restore from the legacy adapter's history dir.
if (!opts.keepHistory) {
this.sessionAdapters.delete(id)
this.sessionRouting.remove(id, adapter)
}
}
@@ -179,9 +184,12 @@ export class DaemonPtyRouter implements IPtyProvider {
async listProcesses(opts?: { deadlineMs?: number }): Promise<PtyProcessInfo[]> {
// Why: runtime exact-stop/liveness flows must fail closed if any adapter
// cannot provide a trustworthy process list.
const results = await Promise.all(
this.allAdapters().map((adapter) => adapter.listProcesses(opts))
)
const adapters = this.allAdapters()
const results = await Promise.all(adapters.map((adapter) => adapter.listProcesses(opts)))
const ambiguousIds = this.sessionRouting.refreshLive(adapters, results)
if (ambiguousIds.length > 0) {
throw this.ambiguousOwnershipError(ambiguousIds)
}
return results.flat()
}
@@ -261,10 +269,10 @@ export class DaemonPtyRouter implements IPtyProvider {
killed.push(id)
}
for (const id of result.alive) {
this.sessionAdapters.set(id, adapter)
this.sessionRouting.add(id, adapter)
}
for (const id of result.killed) {
this.sessionAdapters.delete(id)
this.sessionRouting.remove(id, adapter)
}
}
return { alive, killed }
@@ -318,7 +326,11 @@ export class DaemonPtyRouter implements IPtyProvider {
}
private adapterFor(sessionId: string): DaemonPtyAdapter {
return this.sessionAdapters.get(sessionId) ?? this.current
return this.sessionRouting.get(sessionId) ?? this.current
}
private ambiguousOwnershipError(sessionIds: string[]): Error {
return new Error(`Ambiguous PTY session ownership across daemons: ${sessionIds.join(', ')}`)
}
private allAdapters(): DaemonPtyAdapter[] {
@@ -0,0 +1,79 @@
import type { DaemonPtyAdapter } from './daemon-pty-adapter'
import type { PtyProcessInfo } from '../providers/types'
export class DaemonPtySessionRouting {
private adapters = new Map<string, DaemonPtyAdapter>()
private ambiguousAdapters = new Map<string, Set<DaemonPtyAdapter>>()
get(sessionId: string): DaemonPtyAdapter | undefined {
return this.adapters.get(sessionId)
}
isAmbiguous(sessionId: string): boolean {
return this.ambiguousAdapters.has(sessionId)
}
add(sessionId: string, adapter: DaemonPtyAdapter): void {
const ambiguous = this.ambiguousAdapters.get(sessionId)
if (ambiguous) {
ambiguous.add(adapter)
return
}
const existing = this.adapters.get(sessionId)
if (existing && existing !== adapter) {
this.adapters.delete(sessionId)
this.ambiguousAdapters.set(sessionId, new Set([existing, adapter]))
return
}
this.adapters.set(sessionId, adapter)
}
remove(sessionId: string, adapter: DaemonPtyAdapter): void {
const ambiguous = this.ambiguousAdapters.get(sessionId)
if (ambiguous) {
ambiguous.delete(adapter)
if (ambiguous.size === 1) {
this.ambiguousAdapters.delete(sessionId)
this.adapters.set(sessionId, ambiguous.values().next().value as DaemonPtyAdapter)
} else if (ambiguous.size === 0) {
this.ambiguousAdapters.delete(sessionId)
this.adapters.delete(sessionId)
}
return
}
if (this.adapters.get(sessionId) === adapter) {
this.adapters.delete(sessionId)
}
}
refreshLive(adapters: DaemonPtyAdapter[], results: PtyProcessInfo[][]): string[] {
const owners = new Map<string, Set<DaemonPtyAdapter>>()
for (let index = 0; index < adapters.length; index += 1) {
for (const session of results[index]) {
const sessionOwners = owners.get(session.id) ?? new Set<DaemonPtyAdapter>()
sessionOwners.add(adapters[index])
owners.set(session.id, sessionOwners)
}
}
// Why: unique absent sessions may be sleeping on legacy history, but a
// successful all-daemon inventory can retire stale live ambiguity.
for (const id of this.ambiguousAdapters.keys()) {
if (!owners.has(id)) {
this.ambiguousAdapters.delete(id)
}
}
const ambiguousIds: string[] = []
for (const [id, sessionOwners] of owners) {
if (sessionOwners.size === 1) {
this.ambiguousAdapters.delete(id)
this.adapters.set(id, sessionOwners.values().next().value as DaemonPtyAdapter)
} else {
this.adapters.delete(id)
this.ambiguousAdapters.set(id, sessionOwners)
ambiguousIds.push(id)
}
}
return ambiguousIds.sort()
}
}