From 2a3dbcaf463ebf5d7732a52ed0be10a2234055b3 Mon Sep 17 00:00:00 2001
From: etherman-os <hesapacicam112@gmail.com>
Date: Tue, 30 Jun 2026 15:34:55 -0700
Subject: [PATCH] fix(terminal): prevent corrupted session snapshots during
 init

The init snapshot dumped functions with a line-based filter:

    declare -f | grep -vE '^_[^_]'

That strips a function's *header* line (e.g. `_foo () `) but leaves the
orphaned `{ ... }` body behind, corrupting the snapshot that is sourced
before every command. Sourcing the torn snapshot runs leftover body code
and breaks subsequent commands (intermittent exit 127).

- Filter private (`_`-prefixed) functions by NAME via `declare -F` and
  dump only the wanted whole definitions, so a body is never torn. Guard
  against an empty name list (bare `declare -f` dumps everything).
- Treat a non-zero bootstrap exit code as snapshot-init failure, so
  execution safely falls back to login-shell-per-command mode.
- Add a regression test asserting snapshot_ready stays false when
  bootstrap exits non-zero.

Preserves the atomic-write ($BASHPID temp + mv -f) machinery from #38249.
---
 tests/tools/test_base_environment.py | 16 ++++++++++++++++
 tools/environments/base.py           | 22 ++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/tests/tools/test_base_environment.py b/tests/tools/test_base_environment.py
index bfa20fc5ac1..8d6e6ab7328 100644
--- a/tests/tools/test_base_environment.py
+++ b/tests/tools/test_base_environment.py
@@ -323,6 +323,22 @@ class TestInitSessionFailure:
 
         assert env._snapshot_ready is False
 
+    def test_snapshot_ready_false_on_nonzero_bootstrap_exit(self):
+        """A non-zero bootstrap result should trigger fallback mode."""
+        env = _TestableEnv()
+
+        def mock_run_bash(*args, **kwargs):
+            mock = MagicMock()
+            mock.poll.return_value = 0
+            mock.returncode = 127
+            mock.stdout = iter([])
+            return mock
+
+        env._run_bash = mock_run_bash
+        env.init_session()
+
+        assert env._snapshot_ready is False
+
     def test_login_flag_when_snapshot_not_ready(self):
         """When _snapshot_ready=False, execute() should pass login=True to _run_bash."""
         env = _TestableEnv()
diff --git a/tools/environments/base.py b/tools/environments/base.py
index 409fec71d8b..67107690f1b 100644
--- a/tools/environments/base.py
+++ b/tools/environments/base.py
@@ -357,7 +357,7 @@ class BaseEnvironment(ABC):
         ``_snapshot_ready = True`` so subsequent commands source the snapshot
         instead of running with ``bash -l``.
         """
-        # Full capture: env vars, functions (filtered), aliases, shell options.
+        # Full capture: env vars, functions, aliases, shell options.
         # Restore configured cwd after login shell profile scripts, which may
         # change the working directory (e.g. bashrc `cd ~`).  Without this,
         # pwd -P captures the profile's directory, not terminal.cwd.
@@ -391,7 +391,21 @@ class BaseEnvironment(ABC):
         _snap_tmp = shlex.quote(self._snapshot_path + ".tmp.") + "$BASHPID"
         bootstrap = (
             f"export -p > {_snap_tmp}\n"
-            f"declare -f | grep -vE '^_[^_]' >> {_snap_tmp}\n"
+            # Dump function definitions, filtering out private (``_``-prefixed)
+            # helpers — mainly bash-completion internals (``_git``, ``_make``…)
+            # — by NAME, not by line.  A naive ``declare -f | grep -vE '^_[^_]'``
+            # is line-based: it strips the function *header* line but leaves the
+            # orphaned ``{ … }`` body behind, which corrupts the snapshot and
+            # makes every sourced command fail (e.g. exit 127).  Selecting the
+            # wanted names with ``declare -F`` first, then dumping only those
+            # whole definitions, preserves the filter's intent without ever
+            # tearing a function body.  The non-empty guard matters: bare
+            # ``declare -f`` with no name args dumps ALL functions, so an empty
+            # name list (only private funcs present) would otherwise leak the
+            # very functions we meant to drop.
+            f"__hermes_fns=$(declare -F | awk '{{print $3}}' | grep -vE '^_[^_]') || true\n"
+            f"[ -n \"$__hermes_fns\" ] && declare -f $__hermes_fns "
+            f">> {_snap_tmp} 2>/dev/null || true\n"
             f"alias -p >> {_snap_tmp}\n"
             f"echo 'shopt -s expand_aliases' >> {_snap_tmp}\n"
             f"echo 'set +e' >> {_snap_tmp}\n"
@@ -406,6 +420,10 @@ class BaseEnvironment(ABC):
         try:
             proc = self._run_bash(bootstrap, login=True, timeout=self._snapshot_timeout)
             result = self._wait_for_process(proc, timeout=self._snapshot_timeout)
+            if int(result.get("returncode") or 0) != 0:
+                raise RuntimeError(
+                    f"snapshot bootstrap failed with exit code {result.get('returncode')}"
+                )
             self._snapshot_ready = True
             self._update_cwd(result)
             logger.info(