Merge remote-tracking branch 'origin/main' into pr19002-rebase

.github/workflows/pr.yml

@@ -856,6 +856,7 @@ jobs:
           src/main/agent-hooks/windows-hook-payload-delivery.test.ts
           src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts
           src/main/windows/windows-pty-job.win32.test.ts
+          src/main/windows/windows-msys-job.win32.test.ts
           src/main/windows/windows-host-job.win32.test.ts
           src/main/windows/windows-process-tree-command-line-patch.test.ts
           src/main/windows/windows-process-table-native-addon.win32.test.ts

config/patches/node-pty@1.1.0.patch

@@ -603,7 +603,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..2ae787c5bd4f3eba470584dc658a01a5
  }
  #endif
 diff --git a/src/win/conpty.cc b/src/win/conpty.cc
-index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c97209248e 100644
+index 7b286d3d644c26141df516929703aa6e129df4b2..4b06d18576c807c3d1181a7bd714140c6678cf86 100644
 --- a/src/win/conpty.cc
 +++ b/src/win/conpty.cc
 @@ -18,6 +18,7 @@
@@ -614,7 +614,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
  #include <vector>
  #include <Windows.h>
  #include <strsafe.h>
-@@ -44,12 +45,39 @@ struct pty_baton {
+@@ -44,12 +45,40 @@ struct pty_baton {
    HANDLE hOut;
    HPCON hpc;
  
@@ -630,6 +630,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
 +  // refused to create or assign one (an outer job without breakaway rights),
 +  // in which case callers fall back to their pre-job behaviour.
 +  HANDLE hJob = nullptr;
++  bool allowJobBreakaway = true;
 +
 +  // Orca: teardown needs BOTH the shell's death and an explicit kill() before
 +  // the baton can be freed, so each side records that it has run. Whichever
@@ -655,7 +656,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
  static volatile LONG ptyCounter;
  
  static pty_baton* get_pty_baton(int id) {
-@@ -102,8 +130,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) {
+@@ -102,8 +131,31 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) {
      // Get process exit code.
      GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code));
      // Clean up handles
@@ -689,7 +690,36 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
  
      auto status = tsfn.BlockingCall(exit_event, callback); // In main thread
      switch (status) {
-@@ -409,6 +460,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+@@ -242,6 +294,20 @@
+   return HRESULT_FROM_WIN32(GetLastError());
+ }
+ 
++// Cygwin and MSYS request breakaway for every child whenever the job allows it,
++// so their shells need one that does not. The runtime DLL on the exe's search
++// path is the signal; Git for Windows ships bash.exe in bin\ beside usr\bin\.
++static bool usesCygwinRuntime(const std::wstring& shellpath) {
++  const size_t separator = shellpath.find_last_of(L"\\/");
++  if (separator == std::wstring::npos) return false;
++  const std::wstring directory = shellpath.substr(0, separator + 1);
++  for (const wchar_t* dll : {L"msys-2.0.dll", L"cygwin1.dll"}) {
++    if (path_util::file_exists(directory + dll) ||
++        path_util::file_exists(directory + L"..\\usr\\bin\\" + dll)) return true;
++  }
++  return false;
++}
++
+ static Napi::Value PtyStartProcess(const Napi::CallbackInfo& info) {
+   Napi::Env env(info.Env());
+   Napi::HandleScope scope(env);
+@@ -303,6 +369,7 @@
+     marshal.Set("pty", Napi::Number::New(env, ptyId));
+     ptyHandles.emplace_back(
+         std::make_unique<pty_baton>(ptyId, hIn, hOut, hpc));
++    ptyHandles.back()->allowJobBreakaway = !usesCygwinRuntime(shellpath);
+   } else {
+     throw Napi::Error::New(env, "Cannot launch conpty");
+   }
+@@ -409,6 +476,15 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
      throw errorWithCode(info, "UpdateProcThreadAttribute failed");
    }
  
@@ -705,7 +735,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
    PROCESS_INFORMATION piClient{};
    fSuccess = !!CreateProcessW(
        nullptr,
-@@ -416,7 +476,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+@@ -416,7 +492,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
        nullptr,                      // lpProcessAttributes
        nullptr,                      // lpThreadAttributes
        false,                        // bInheritHandles VERY IMPORTANT that this is false
@@ -717,7 +747,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
        envArg,                       // lpEnvironment
        mutableCwd.get(),             // lpCurrentDirectory
        &siEx.StartupInfo,            // lpStartupInfo
-@@ -426,8 +489,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+@@ -426,8 +505,48 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
      throw errorWithCode(info, "Cannot create process");
    }
  
@@ -735,13 +765,14 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
 +  // EXPLICIT teardown exact, not to redefine what a clean exit means.
 +  HANDLE hJob = CreateJobObjectW(nullptr, nullptr);
 +  if (hJob != nullptr) {
-+    // Why BREAKAWAY_OK and not a bare job: with no limits set, a child asking
-+    // for CREATE_BREAKAWAY_FROM_JOB is refused with ERROR_ACCESS_DENIED.
-+    // Installers, msiexec and some updater and service-control paths spawn that
-+    // way deliberately, so a bare job breaks them ONLY inside an Orca terminal.
-+    // With this flag a child has to ask, so ordinary descendants stay owned.
++    // Native shells retain explicit breakaway for installers and updaters.
++    // Cygwin/MSYS shells take it automatically for ordinary children whenever
++    // this flag is present, so they get strict per-PTY membership instead.
++    // Explicit breakaway requests inside such a pane are consequently denied;
++    // ordinary backgrounding and clean shell exit remain supported.
 +    JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobLimits{};
-+    jobLimits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_BREAKAWAY_OK;
++    jobLimits.BasicLimitInformation.LimitFlags =
++        handle->allowJobBreakaway ? JOB_OBJECT_LIMIT_BREAKAWAY_OK : 0;
 +    if (!SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &jobLimits, sizeof(jobLimits)) ||
 +        !AssignProcessToJobObject(hJob, piClient.hProcess)) {
 +      // Why tolerate failure: an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK
@@ -767,7 +798,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
    if (useConptyDll && fLoadedDll)
    {
      PFNRELEASEPSEUDOCONSOLE const pfnReleasePseudoConsole = (PFNRELEASEPSEUDOCONSOLE)GetProcAddress(
-@@ -440,6 +542,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
+@@ -440,6 +559,8 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) {
  
    // Update handle
    handle->hShell = piClient.hProcess;
@@ -776,11 +807,16 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
  
    // Close the thread handle to avoid resource leak
    CloseHandle(piClient.hThread);
-@@ -544,29 +648,215 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
+@@ -544,27 +665,213 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) {
    int id = info[0].As<Napi::Number>().Int32Value();
    const bool useConptyDll = info[1].As<Napi::Boolean>().Value();
  
 -  const pty_baton* handle = get_pty_baton(id);
+-
+-  if (handle != nullptr) {
+-    HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
+-    bool fLoadedDll = hLibrary != nullptr;
+-    if (fLoadedDll)
 +  // Orca: resolve the DLL BEFORE touching any baton state, for the same reason
 +  // PtyConnect does it before creating anything. LoadConptyDll throws when
 +  // conpty.dll is missing, and a throw after consoleClosed was set would strand
@@ -794,18 +830,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
 +      (HMODULE)hLibrary,
 +      useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
 +  }
- 
--  if (handle != nullptr) {
--    HANDLE hLibrary = LoadConptyDll(info, useConptyDll);
--    bool fLoadedDll = hLibrary != nullptr;
--    if (fLoadedDll)
--    {
--      PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress(
--        (HMODULE)hLibrary,
--        useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
--      if (pfnClosePseudoConsole)
--      {
--        pfnClosePseudoConsole(handle->hpc);
++
 +  // Orca: the baton now outlives the shell, so this runs on a self-exited pty
 +  // too -- that is the whole point. Take what we need under the lock: the
 +  // watcher thread nulls hShell the moment the shell dies, and TerminateProcess
@@ -841,18 +866,26 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
 +        const bool removed = remove_pty_baton(id);
 +        assert(removed);
 +        (void)removed;
-       }
++      }
 +      // Else the shell is still running and the watcher frees the baton.
-     }
--    if (useConptyDll) {
--      TerminateProcess(handle->hShell, 1);
++    }
 +  }
 +
 +  // Why outside the lock: ClosePseudoConsole blocks until the conout side has
 +  // drained, and the watcher must be able to take the lock while it does.
 +  if (owed) {
 +    if (pfnClosePseudoConsole)
-+    {
+     {
+-      PFNCLOSEPSEUDOCONSOLE const pfnClosePseudoConsole = (PFNCLOSEPSEUDOCONSOLE)GetProcAddress(
+-        (HMODULE)hLibrary,
+-        useConptyDll ? "ConptyClosePseudoConsole" : "ClosePseudoConsole");
+-      if (pfnClosePseudoConsole)
+-      {
+-        pfnClosePseudoConsole(handle->hpc);
+-      }
+-    }
+-    if (useConptyDll) {
+-      TerminateProcess(handle->hShell, 1);
 +      pfnClosePseudoConsole(hpc);
 +    }
 +    if (hShellDup != nullptr) {
@@ -862,8 +895,8 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
    }
  
    return env.Undefined();
- }
- 
++}
++
 +/**
 + * Orca: confirm a baton really is the pty the caller means.
 + *
@@ -1001,12 +1034,10 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4aed260dd68e6a171dcfd349e9a7c5c9
 +  }
 +  hHostJob = job;
 +  return Napi::Boolean::New(env, true);
-+}
-+
+ }
+ 
  /**
- * Init
- */
-@@ -577,6 +867,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
+@@ -577,6 +884,9 @@ Napi::Object init(Napi::Env env, Napi::Object exports) {
    exports.Set("resize", Napi::Function::New(env, PtyResize));
    exports.Set("clear", Napi::Function::New(env, PtyClear));
    exports.Set("kill", Napi::Function::New(env, PtyKill));

config/scripts/pr-code-change-scope.mjs

@@ -224,6 +224,7 @@ const WINDOWS_PACKAGE_TESTS = [
   'src/main/agent-hooks/windows-hook-payload-delivery.test.ts',
   'src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts',
   'src/main/windows/windows-pty-job.win32.test.ts',
+  'src/main/windows/windows-msys-job.win32.test.ts',
   'src/main/windows/windows-host-job.win32.test.ts',
   'src/main/windows/windows-process-tree-command-line-patch.test.ts',
   'src/main/windows/windows-process-table-native-addon.win32.test.ts',

docs/reference/windows-process-enumeration.md

@@ -575,6 +575,20 @@ running, so typing `exit` in a pane reaped a `start /b` server that used to
 survive. The job exists to make an _explicit_ teardown exact, not to redefine
 what a clean exit means.
 
+Git Bash needs one additional restriction. The Cygwin runtime — and the MSYS2
+fork of it that Git for Windows ships — reads `JOB_OBJECT_LIMIT_BREAKAWAY_OK`
+off its own job and then adds `CREATE_BREAKAWAY_FROM_JOB` to **every** child it
+spawns when that flag is set (`spawn.cc`, there since 2011), so offering
+breakaway hands the whole tree its escape. The per-PTY job therefore omits
+`BREAKAWAY_OK` whenever `msys-2.0.dll` or `cygwin1.dll` sits on the shell's DLL
+search path — beside the executable, or under `usr/bin` for Git's `bin`
+launcher. Native shells keep explicit breakaway. Denying it costs Cygwin
+nothing, because it *pre-checks* the limit rather than retrying, so no spawn
+fails; but a *native* program that passes `CREATE_BREAKAWAY_FROM_JOB` itself
+inside such a pane now gets `ERROR_ACCESS_DENIED`. `nohup` and `disown` are
+unaffected — they are Cygwin signal/session concepts, unrelated to job
+membership. The daemon's host job is unchanged.
+
 Reaping a dead daemon's shells (#9195, #10415) is therefore a **second, nested
 job**, not this one. The terminal daemon assigns itself to a kill-on-close job
 at startup (`assignHostProcessToKillOnCloseJob`); children inherit membership,

pnpm-lock.yaml

@@ -116,7 +116,7 @@ patchedDependencies:
   '@xterm/addon-webgl@0.20.0-beta.299': 94687e89a0115e6e6aa102837f986debdc029c091527ee5eb4a4e17ceaf9473e
   '@xterm/xterm@6.1.0-beta.303': 98756bcedc402bcdb7c6ab7b015d2e59cd18e97b03a2c06a27e95bb3ba429d9d
   lint-staged@16.4.0: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673
-  node-pty@1.1.0: 7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1
+  node-pty@1.1.0: bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615
 
 importers:
 
@@ -160,7 +160,7 @@ importers:
         version: 3.3.1
       node-pty:
         specifier: ^1.1.0
-        version: 1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1)
+        version: 1.1.0(patch_hash=bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615)
       posthog-node:
         specifier: ^5.33.3
         version: 5.33.3
@@ -12285,7 +12285,7 @@ snapshots:
 
   node-int64@0.4.0: {}
 
-  node-pty@1.1.0(patch_hash=7cc9d45f3d2c38f142490d0805e75db55f0eef5174ad41c4b52abc5fbe079ad1):
+  node-pty@1.1.0(patch_hash=bac3a53fb15efc9b3b944fbe3c4718b5174a0b3bd6ead84e21975edad4bc6615):
     dependencies:
       node-addon-api: 7.1.1
 

src/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.ts

@@ -138,6 +138,11 @@ describe('committed adopting create RPC replay', () => {
       undefined,
       { prepareCodexStructuredLaunch: selectAccountHome }
     )
+    // The structured surface is settings-gated for every caller, not just mobile; this test
+    // probes durable-identity replay, which only runs once the gate admits the call.
+    vi.spyOn(runtime, 'getClientSettings').mockReturnValue({
+      experimentalStructuredNativeChat: true
+    } as ReturnType<OrcaRuntimeService['getClientSettings']>)
     vi.spyOn(runtime, 'getStructuredAgentSessionCreateSupport').mockResolvedValue({
       supported: true
     })

src/main/windows/windows-msys-job.win32.test.ts

@@ -0,0 +1,65 @@
+import { mkdtempSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { describe, expect, it, vi } from 'vitest'
+import { removeTreeSync } from '../../shared/windows-transient-lock-removal'
+import { resolveGitBashPath } from '../git-bash'
+import { quotePosixShell } from '../../shared/wsl-login-shell-command'
+import { listPtyJobProcessIds, terminatePtyJob } from './windows-pty-job'
+
+const describeOnWindows = process.platform === 'win32' ? describe : describe.skip
+
+function isAlive(pid: number): boolean {
+  try {
+    process.kill(pid, 0)
+    return true
+  } catch (error) {
+    return (error as NodeJS.ErrnoException).code === 'EPERM'
+  }
+}
+
+describeOnWindows('MSYS terminal job ownership', () => {
+  it('retains and terminates a child across Git Bash shell replacement', async () => {
+    const shell = resolveGitBashPath()
+    expect(shell, 'Git for Windows must be installed on the native test runner').not.toBeNull()
+    const directory = mkdtempSync(join(tmpdir(), 'orca-msys-job-'))
+    const script = join(directory, 'owned-child.js')
+    writeFileSync(
+      script,
+      "console.log('MSYS_OWNED_CHILD=' + process.pid); setInterval(() => {}, 1000)\n"
+    )
+    const pty = await import('node-pty')
+    const proc = pty.spawn(shell!, ['-c', 'exec "$BASH" --noprofile --norc -i'], {
+      cwd: tmpdir(),
+      cols: 120,
+      rows: 30,
+      useConptyDll: true
+    })
+    let output = ''
+    let childPid: number | undefined
+    proc.onData((chunk) => {
+      output += chunk
+      const match = /MSYS_OWNED_CHILD=(\d+)/.exec(output)
+      if (match) {
+        childPid = Number(match[1])
+      }
+    })
+    try {
+      proc.write(
+        `${quotePosixShell(process.execPath.replace(/\\/g, '/'))} ${quotePosixShell(script.replace(/\\/g, '/'))}\r`
+      )
+      await vi.waitFor(() => expect(childPid).toBeDefined(), { timeout: 15_000 })
+      expect(isAlive(childPid!)).toBe(true)
+      expect(listPtyJobProcessIds(proc)).toContain(childPid)
+      expect(terminatePtyJob(proc)).toBe('terminated')
+      await vi.waitFor(() => expect(isAlive(childPid!)).toBe(false), { timeout: 5_000 })
+    } finally {
+      // The failing baseline can leave this exact fixture child outside the job.
+      if (childPid && isAlive(childPid)) {
+        process.kill(childPid)
+      }
+      proc.kill()
+      removeTreeSync(directory)
+    }
+  }, 30_000)
+})