fix: confirm proxy exit before disconnect

test/proxy-launch.test.js

@@ -24,6 +24,8 @@ class FakeChild extends EventEmitter {
     this.killResult = killResult;
     this.killCalls = [];
     this.killError = null;
+    this.exitCode = null;
+    this.signalCode = null;
   }
 
   kill(signal) {
@@ -33,7 +35,13 @@ class FakeChild extends EventEmitter {
   }
 }
 
-function createHarness({ helperExists = true, children = [new FakeChild(4101)] } = {}) {
+function createHarness({
+  helperExists = true,
+  children = [new FakeChild(4101)],
+  stopTimeoutMs = 5_000,
+  setTimeout = global.setTimeout,
+  clearTimeout = global.clearTimeout,
+} = {}) {
   const calls = [];
   let childIndex = 0;
   const spawn = (...args) => {
@@ -47,7 +55,10 @@ function createHarness({ helperExists = true, children = [new FakeChild(4101)] }
     fs: fsStub,
     spawn,
     platform: 'win32',
-    now: () => 1_777_777_777_777
+    now: () => 1_777_777_777_777,
+    stopTimeoutMs,
+    setTimeout,
+    clearTimeout,
   });
   return { manager, calls, children };
 }
@@ -239,32 +250,38 @@ test('tracks only safe process metadata and never exposes usernames', () => {
   assert.doesNotMatch(JSON.stringify(tracked), /proxy\.operator/);
 });
 
-test('stopping one tracked process leaves the other tracked process alive', () => {
+test('stopping one tracked process waits for confirmed exit and leaves the other alive', async () => {
   const first = new FakeChild(4101);
   const second = new FakeChild(4102);
   const { manager } = createHarness({ children: [first, second] });
   manager.launchProxy(validRequest());
   manager.launchProxy(validRequest({ deviceId: '123e4567-e89b-42d3-a456-426614174001' }));
 
-  const result = manager.stopProxy(4101);
+  const stopping = manager.stopProxy(4101);
+  assert.deepEqual(manager.listTrackedProxies().map((item) => [item.processId, item.status]), [
+    [4101, 'stopping'],
+    [4102, 'running'],
+  ]);
+  first.emit('exit', 0, 'SIGTERM');
+  const result = await stopping;
 
   assert.deepEqual(result, {
     success: true,
     processId: 4101,
     deviceId: VALID_DEVICE_ID,
-    status: 'stop-requested'
+    status: 'stopped'
   });
   assert.deepEqual(first.killCalls, ['SIGTERM']);
   assert.deepEqual(second.killCalls, []);
   assert.deepEqual(manager.listTrackedProxies().map((item) => item.processId), [4102]);
 });
 
-test('never stops an untracked PID', () => {
+test('never stops an untracked PID', async () => {
   const child = new FakeChild(4101);
   const { manager } = createHarness({ children: [child] });
   manager.launchProxy(validRequest());
 
-  assert.deepEqual(manager.stopProxy(9999), {
+  assert.deepEqual(await manager.stopProxy(9999), {
     success: false,
     processId: 9999,
     status: 'not-tracked'
@@ -272,21 +289,22 @@ test('never stops an untracked PID', () => {
   assert.deepEqual(child.killCalls, []);
 });
 
-test('reports already-exited and permission-denied states honestly', () => {
+test('reports already-exited and permission-denied states honestly', async () => {
   const exited = new FakeChild(4101, false);
+  exited.exitCode = 0;
   const denied = new FakeChild(4102);
   denied.killError = Object.assign(new Error('operation not permitted'), { code: 'EPERM' });
   const { manager } = createHarness({ children: [exited, denied] });
   manager.launchProxy(validRequest());
   manager.launchProxy(validRequest({ deviceId: '123e4567-e89b-42d3-a456-426614174001' }));
 
-  assert.deepEqual(manager.stopProxy(4101), {
+  assert.deepEqual(await manager.stopProxy(4101), {
     success: true,
     processId: 4101,
     deviceId: VALID_DEVICE_ID,
     status: 'already-exited'
   });
-  assert.deepEqual(manager.stopProxy(4102), {
+  assert.deepEqual(await manager.stopProxy(4102), {
     success: false,
     processId: 4102,
     deviceId: '123e4567-e89b-42d3-a456-426614174001',
@@ -296,6 +314,60 @@ test('reports already-exited and permission-denied states honestly', () => {
   assert.deepEqual(manager.listTrackedProxies().map((item) => item.processId), [4102]);
 });
 
+test('kill true is only a request and times out while the child remains tracked as stopping', async () => {
+  let timeoutCallback;
+  const child = new FakeChild(4101, true);
+  const { manager } = createHarness({
+    children: [child],
+    stopTimeoutMs: 25,
+    setTimeout(callback, delay) {
+      assert.equal(delay, 25);
+      timeoutCallback = callback;
+      return 123;
+    },
+    clearTimeout() {},
+  });
+  manager.launchProxy(validRequest());
+
+  const stopping = manager.stopProxy(4101);
+  assert.deepEqual(manager.listTrackedProxies().map(({ processId, status }) => ({ processId, status })), [
+    { processId: 4101, status: 'stopping' },
+  ]);
+  timeoutCallback();
+
+  assert.deepEqual(await stopping, {
+    success: false,
+    processId: 4101,
+    deviceId: VALID_DEVICE_ID,
+    status: 'stop-timeout',
+    message: 'Timed out waiting for the tracked proxy process to exit.',
+  });
+  assert.equal(manager.listTrackedProxies()[0].status, 'stopping');
+});
+
+test('kill false does not claim success without confirmed exit', async () => {
+  const child = new FakeChild(4101, false);
+  const { manager } = createHarness({ children: [child] });
+  manager.launchProxy(validRequest());
+
+  assert.deepEqual(await manager.stopProxy(4101), {
+    success: false,
+    processId: 4101,
+    deviceId: VALID_DEVICE_ID,
+    status: 'stop-failed',
+    message: 'Unable to confirm that the tracked proxy process exited.',
+  });
+  assert.equal(manager.listTrackedProxies()[0].status, 'stopping');
+});
+
+test('generic child errors retain tracking unless process exit is confirmed', () => {
+  const child = new FakeChild(4101);
+  const { manager } = createHarness({ children: [child] });
+  manager.launchProxy(validRequest());
+  child.emit('error', new Error('transient process error'));
+  assert.deepEqual(manager.listTrackedProxies().map((entry) => entry.processId), [4101]);
+});
+
 test('exit events remove only the matching owned child', () => {
   const original = new FakeChild(4101);
   const replacement = new FakeChild(4101);

test/runtime-contract.test.js

@@ -275,13 +275,13 @@ test('disconnect stops every owned proxy before clearing the Alta session', asyn
       },
     },
   });
-  const state = runtime.disconnect();
+  const state = await runtime.disconnect();
   assert.deepEqual(stopped, [4101, 4102]);
   assert.equal(state.connected, false);
   assert.deepEqual(state.activeProxies, []);
 });
 
-test('disconnect reports failure truthfully and retains session when an owned proxy cannot stop', () => {
+test('disconnect reports failure truthfully and retains session when an owned proxy cannot stop', async () => {
   const sessionStore = createSessionStore();
   sessionStore.establish('https://customer.avasecurity.com', 'synthetic-cookie');
   const tracked = [{ processId: 4101, deviceId: '550e8400-e29b-41d4-a716-446655440000', status: 'running', startedAt: 1 }];
@@ -293,12 +293,52 @@ test('disconnect reports failure truthfully and retains session when an owned pr
       stopProxy() { return { success: false, processId: 4101, status: 'permission-denied' }; },
     },
   });
-  const state = runtime.disconnect();
+  const state = await runtime.disconnect();
   assert.equal(state.success, false);
   assert.equal(state.connected, true);
   assert.deepEqual(state.activeProxies.map((entry) => entry.processId), [4101]);
 });
 
+test('kill request without exit keeps proxy tracked and session connected until confirmed exit', async () => {
+  const { EventEmitter } = require('node:events');
+  const { createProxyManager } = require('../src/proxy-launch');
+  const sessionStore = createSessionStore();
+  sessionStore.establish('https://customer.avasecurity.com', 'synthetic-cookie');
+  const deviceId = '550e8400-e29b-41d4-a716-446655440000';
+  const child = new EventEmitter();
+  child.pid = 4101;
+  child.exitCode = null;
+  child.signalCode = null;
+  child.kill = () => true;
+  let timeoutCallback;
+  const proxyManager = createProxyManager({
+    appDirectory: 'C:\\Program Files\\Alta Proxy Tool',
+    fs: { existsSync: () => true },
+    spawn: () => child,
+    platform: 'win32',
+    stopTimeoutMs: 10,
+    setTimeout(callback) { timeoutCallback = callback; return 1; },
+    clearTimeout() {},
+  });
+  proxyManager.launchProxy({ deploymentHost: 'customer.avasecurity.com', username: 'operator@example.com', deviceId });
+  const runtime = new AppRuntime({ sessionStore, altaClient: {}, proxyManager });
+
+  const firstDisconnect = runtime.disconnect();
+  assert.equal(proxyManager.listTrackedProxies()[0].status, 'stopping');
+  timeoutCallback();
+  const failed = await firstDisconnect;
+  assert.equal(failed.success, false);
+  assert.equal(failed.connected, true);
+  assert.deepEqual(failed.activeProxies, [{ deviceId, processId: 4101 }]);
+
+  child.exitCode = 0;
+  child.emit('exit', 0, null);
+  const disconnected = await runtime.disconnect();
+  assert.equal(disconnected.success, true);
+  assert.equal(disconnected.connected, false);
+  assert.deepEqual(disconnected.activeProxies, []);
+});
+
 test('update runtime is check-only and opens only the fixed GitPeji releases page', async () => {
   const opened = [];
   const runtime = new AppRuntime({
@@ -340,6 +380,24 @@ test('preload and renderer expose only narrow, credential-free contracts', () =>
   assert.match(html, /Bridge Pairing/);
 });
 
+test('Electron quit waits for confirmed cleanup and reports a safe failure instead of disposing', () => {
+  const main = read('main.js');
+  const beforeQuit = main.slice(main.indexOf("app.on('before-quit'"), main.indexOf("app.on('window-all-closed'"));
+  const preventIndex = beforeQuit.indexOf('event.preventDefault()');
+  const disconnectIndex = beforeQuit.indexOf('await runtime.disconnect()');
+  const activeCheckIndex = beforeQuit.indexOf('state.activeProxies.length > 0');
+  const disposeIndex = beforeQuit.indexOf('runtime.sessionStore.dispose()');
+  const confirmedQuitIndex = beforeQuit.indexOf('allowConfirmedQuit = true');
+  const quitIndex = beforeQuit.indexOf('app.quit()');
+
+  assert.ok(preventIndex >= 0 && preventIndex < disconnectIndex);
+  assert.ok(disconnectIndex < activeCheckIndex && activeCheckIndex < disposeIndex);
+  assert.ok(disposeIndex < confirmedQuitIndex && confirmedQuitIndex < quitIndex);
+  assert.match(main, /dialog\.showMessageBox/);
+  assert.match(main, /could not confirm that every proxy process exited/i);
+  assert.doesNotMatch(beforeQuit, /taskkill|pkill|wmic/i);
+});
+
 test('source policy removes legacy credential IPC, shell launch, broad kill, and executable updater', () => {
   const production = ['main.js', 'preload.js', 'renderer.js', 'index.html', 'src/electron-runtime.js']
     .map(read).join('\n');