diff --git a/tests/tools/test_local_shell_init.py b/tests/tools/test_local_shell_init.py index f1e4f5b0451..178c02e6a95 100644 --- a/tests/tools/test_local_shell_init.py +++ b/tests/tools/test_local_shell_init.py @@ -194,12 +194,12 @@ class TestSnapshotEndToEnd: env = LocalEnvironment(cwd=str(tmp_path), timeout=15) try: first = env.execute( - 'export HERMES_SESSION_ENV_PROBE="sticky"; ' + 'export HERMES_STICKY_ENV_PROBE="sticky"; ' 'export PATH="/tmp/hermes-session-bin:$PATH"; ' - 'echo "first=$HERMES_SESSION_ENV_PROBE"' + 'echo "first=$HERMES_STICKY_ENV_PROBE"' ) second = env.execute( - 'echo "second=$HERMES_SESSION_ENV_PROBE"; echo "PATH=$PATH"' + 'echo "second=$HERMES_STICKY_ENV_PROBE"; echo "PATH=$PATH"' ) finally: env.cleanup() diff --git a/tests/tools/test_snapshot_session_id_leak.py b/tests/tools/test_snapshot_session_id_leak.py index d48f4f44968..c48555988ae 100644 --- a/tests/tools/test_snapshot_session_id_leak.py +++ b/tests/tools/test_snapshot_session_id_leak.py @@ -58,7 +58,14 @@ def test_export_snippet_shape(): assert "export -p" in snippet assert "grep -vE" in snippet assert "/tmp/snap.tmp.$BASHPID" in snippet - assert snippet.rstrip().endswith("|| true") + # The redirection must be attached to a brace group wrapping the pipeline, + # NOT to the grep segment: a redirect on grep expands $BASHPID inside + # grep's pipeline subshell (a different PID than the parent shell that + # expands the follow-up ``mv`` operand), silently orphaning the dump and + # breaking snapshot env persistence entirely. + assert snippet.lstrip().startswith("{ ") + assert "|| true; }" in snippet + assert snippet.rstrip().endswith("> /tmp/snap.tmp.$BASHPID") # --------------------------------------------------------------------------- diff --git a/tools/environments/base.py b/tools/environments/base.py index 7199079fb24..74c33aff753 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -412,9 +412,20 @@ def _export_dump_excluding_session_vars(tmp_path: str) -> str: persist across sessions in the shared snapshot. ``grep -vE`` returns exit 1 when it filters everything, so ``|| true`` keeps the pipeline's success contract intact for the callers that chain on it. + + The pipeline MUST be wrapped in a brace group with the redirection applied + to the group, not to the last pipeline segment. *tmp_path* typically embeds + ``$BASHPID`` for concurrency-safe temp names; a redirection attached + directly to ``grep`` is expanded inside grep's own pipeline subshell, where + ``$BASHPID`` resolves to the grep subshell's PID — while the caller's + follow-up ``mv $tmp`` expands in the parent shell to a DIFFERENT PID. The + dump then lands in an orphaned temp file and the snapshot silently never + updates (all exported-env persistence breaks). The brace-group redirect is + expanded in the current shell, keeping both expansions consistent. """ return ( - f"export -p | grep -vE '{_SNAPSHOT_EXCLUDED_ENV_REGEX}' > {tmp_path} || true" + f"{{ export -p | grep -vE '{_SNAPSHOT_EXCLUDED_ENV_REGEX}' || true; }} " + f"> {tmp_path}" ) @@ -678,9 +689,14 @@ class BaseEnvironment(ABC): # Chain mv on the export succeeding so a failed/partial dump never # replaces a good snapshot; drop the temp on failure so it isn't # orphaned (cleaned up wholesale in LocalEnvironment.cleanup too). + # NOTE: the redirection must be attached to a brace group, not to the + # grep pipeline segment — ``_snap_tmp`` embeds ``$BASHPID``, and a + # redirect on grep is expanded inside grep's pipeline subshell (a + # different PID than the parent shell that expands the ``mv`` operand), + # silently orphaning the dump. See _export_dump_excluding_session_vars. if self._snapshot_ready: parts.append( - f"{{ export -p | grep -vE '{_SNAPSHOT_EXCLUDED_ENV_REGEX}' > {_snap_tmp} " + f"{{ {_export_dump_excluding_session_vars(_snap_tmp)} " f"&& mv -f {_snap_tmp} {_quoted_snap}; }} " f"2>/dev/null || rm -f {_snap_tmp} 2>/dev/null || true" )