fix(shell): drop POSIX quote-wrap in _shell_escape; strip CR in safe_eval (#84)

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "remember",
   "description": "Continuous memory for Claude Code. Extracts, summarizes, and compresses conversations into tiered daily logs. Claude remembers what you did yesterday.",
-  "version": "0.7.2",
+  "version": "0.7.3",
   "author": {
     "name": "Digital Process Tools"
   },

CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.7.3] — Windows save pipeline shell↔Python bridge
+
+### Fixed
+
+- **Save pipeline broken on Windows / Git Bash** ([#84](https://github.com/Digital-Process-Tools/claude-remember/issues/84)) — the shell↔Python bridge had two mismatched halves: `pipeline.shell._shell_escape` single-quote-wrapped values per POSIX `eval` convention, but `safe_eval` in `scripts/log.sh` assigned verbatim via `printf -v` (no shell expansion). On Linux, temp paths contain no shell-unsafe chars so the escaper returned them unquoted — invisible. On Windows, backslash paths got quoted, then stored with literal quotes, then `open()` failed with `OSError: [Errno 22]`. Plus `safe_eval` did not strip CR, so Python's `\r\n` line endings on Windows corrupted integer values and broke `[ -eq ]` tests in `save-session.sh`. Fix: `_shell_escape` now emits verbatim (raises on newline); `safe_eval` strips trailing `\r`; redundant override in `detect-tools.sh` removed (`log.sh` is single source of truth). Issue reported by [@qzftsh7f44-design](https://github.com/qzftsh7f44-design).
+
+### Tests
+
+- New `tests/test_safe_eval_seam.py` pins the Python↔bash roundtrip contract — parametrized across Linux paths, Windows backslash paths, spaces, single quotes. Closes the seam gap CI was blind to (both sides were unit-tested in isolation, never together).
+- 391 tests, 99% coverage.
+
 ## [0.7.1] — Windows portability fixes
 
 ### Fixed

README.md

@@ -6,7 +6,7 @@
 [![Python](https://img.shields.io/badge/python-3.9%2B-blue)](https://www.python.org/)
 [![OS](https://img.shields.io/badge/tested%20on-Linux%20%7C%20macOS%20%7C%20Windows-blue)](https://github.com/Digital-Process-Tools/claude-remember/actions/workflows/tests.yml)
 [![License](https://img.shields.io/badge/license-Community-brightgreen)](LICENSE)
-[![Version](https://img.shields.io/badge/version-0.7.2-orange)](.claude-plugin/plugin.json)
+[![Version](https://img.shields.io/badge/version-0.7.3-orange)](.claude-plugin/plugin.json)
 
 Claude Code starts every session blank. It doesn't know what you worked on yesterday, what conventions your team follows, or what mistakes it already made. You re-explain everything, every time.
 

pipeline/shell.py

@@ -35,16 +35,29 @@
 
 
 def _shell_escape(value: str) -> str:
-    """Escape a string for safe shell ``eval`` by single-quote wrapping.
+    """Emit a value for the shell variable bridge consumed by ``safe_eval``.
+
+    ``scripts/log.sh:safe_eval`` parses ``KEY=VALUE`` lines and assigns
+    ``VALUE`` verbatim via ``printf -v`` — no shell expansion, no ``eval``.
+    The only constraint is that ``VALUE`` must not contain a newline
+    (the parser is line-oriented).
+
+    Earlier versions single-quote-wrapped per POSIX ``eval`` convention,
+    which broke on Windows: paths with backslashes were quoted, but
+    ``safe_eval``'s verbatim assignment kept the quotes literal (issue #84).
 
     Args:
-        value: Raw string that may contain spaces, quotes, or special chars.
+        value: Raw string. Must not contain newlines.
 
     Returns:
-        Single-quoted string safe for shell evaluation. Internal single
-        quotes are escaped via the ``'\\''`` idiom.
+        The value as-is — emission is verbatim to match parser semantics.
+
+    Raises:
+        ValueError: If ``value`` contains a newline character.
     """
-    return "'" + value.replace("'", "'\\''") + "'"
+    if "\n" in value or "\r" in value:
+        raise ValueError("shell-bridged values must not contain newlines")
+    return value
 
 
 def cmd_extract(session_id: str, project_dir: str) -> None:

scripts/detect-tools.sh

@@ -78,20 +78,10 @@ PYEOF
 fi
 export JQ
 
-# --- CRLF-safe safe_eval ---
-# Override safe_eval to strip \r from lines before eval.
-# On Windows (Git Bash), Python outputs \r\n. read -r keeps the \r,
-# which corrupts variable values (e.g., POSITION="42\r" breaks arithmetic).
-safe_eval() {
-    while IFS= read -r line; do
-        line="${line%$'\r'}"
-        if [[ "$line" =~ ^([A-Z_][A-Z0-9_]*)=(.*)$ ]]; then
-            local _key="${BASH_REMATCH[1]}"
-            local _val="${BASH_REMATCH[2]}"
-            printf -v "$_key" '%s' "$_val"
-        fi
-    done
-}
+# Note: safe_eval lives in log.sh (single source of truth). It strips CR
+# from CRLF input — needed because Python on Windows emits \r\n (issue #84).
+# Earlier versions overrode safe_eval here as a Windows-CRLF patch — removed
+# now that log.sh carries the fix and is sourced after this file.
 
 # --- CRLF-safe session dir slug ---
 # Replaces all non-alphanumeric chars with dashes. Must match Claude Code's

scripts/log.sh

@@ -138,6 +138,9 @@ log_tokens() {
 #   safe_eval <<< "$(python3 -m pipeline.shell extract ...)"
 safe_eval() {
     while IFS= read -r line; do
+        # Strip trailing CR — Python on Windows emits \r\n, which corrupts
+        # numeric values and trips integer tests downstream (issue #84).
+        line="${line%$'\r'}"
         if [[ "$line" =~ ^([A-Z_][A-Z0-9_]*)=(.*)$ ]]; then
             local _key="${BASH_REMATCH[1]}"
             local _val="${BASH_REMATCH[2]}"

tests/test_path_resolution.py

@@ -1334,7 +1334,8 @@ def test_safe_eval_with_lf(self):
         """safe_eval works with normal LF line endings."""
         result = subprocess.run(
             ["bash", "-c",
-             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; '
+             f'export PROJECT_DIR="{REPO_ROOT}" PIPELINE_DIR="{REPO_ROOT}" REMEMBER_DIR="{REPO_ROOT}/.remember-test"; '
+             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; source "{REPO_ROOT}/scripts/log.sh"; '
              'safe_eval <<< "FOO=bar"; echo "FOO=$FOO"'],
             capture_output=True, text=True,
         )
@@ -1344,7 +1345,8 @@ def test_safe_eval_with_crlf(self):
         """safe_eval strips \\r from CRLF lines — values are clean (fixed via detect-tools.sh)."""
         result = subprocess.run(
             ["bash", "-c",
-             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; '
+             f'export PROJECT_DIR="{REPO_ROOT}" PIPELINE_DIR="{REPO_ROOT}" REMEMBER_DIR="{REPO_ROOT}/.remember-test"; '
+             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; source "{REPO_ROOT}/scripts/log.sh"; '
              'safe_eval < <(printf "FOO=bar\\r\\n"); '
              'echo -n "$FOO" | xxd | grep -q "0d" && echo "CORRUPTED" || echo "CLEAN"'],
             capture_output=True, text=True,
@@ -1357,7 +1359,8 @@ def test_safe_eval_crlf_arithmetic(self):
         """CRLF-safe safe_eval: numeric values work in arithmetic."""
         result = subprocess.run(
             ["bash", "-c",
-             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; '
+             f'export PROJECT_DIR="{REPO_ROOT}" PIPELINE_DIR="{REPO_ROOT}" REMEMBER_DIR="{REPO_ROOT}/.remember-test"; '
+             f'source "{REPO_ROOT}/scripts/detect-tools.sh"; source "{REPO_ROOT}/scripts/log.sh"; '
              'safe_eval < <(printf "NUM=42\\r\\n"); '
              'echo "RESULT=$((NUM + 1))"'],
             capture_output=True, text=True,

tests/test_safe_eval_seam.py

@@ -0,0 +1,137 @@
+"""Tests for the shell↔Python variable-bridge seam (issue #84).
+
+Pre-fix, `pipeline.shell._shell_escape` single-quote-wrapped values per
+POSIX `eval` convention, while `safe_eval` in `scripts/log.sh` assigned
+verbatim via `printf -v` — no shell expansion. Mismatched halves: Linux
+temp paths contained no shell-unsafe chars so the escaper returned them
+unquoted and the bug was invisible; Windows backslash paths got quoted
+and the literal quotes survived into the variable, breaking downstream
+`open()` with `OSError: [Errno 22]`.
+
+Also pre-fix, `log.sh`'s `safe_eval` did not strip CR — Python on
+Windows emits `\\r\\n`, so values kept a trailing `\\r` and broke the
+integer tests in `save-session.sh`.
+
+Post-fix: `_shell_escape` is verbatim (raises on newline), `safe_eval`
+strips CR, and the redundant override in `detect-tools.sh` is removed
+(`log.sh` is the single source of truth).
+
+These tests pin the contract both halves must satisfy, including a
+parametrized Python→shell roundtrip across Linux/Windows path shapes.
+"""
+
+from __future__ import annotations
+
+import os
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+LOG_SH = REPO_ROOT / "scripts" / "log.sh"
+DETECT_SH = REPO_ROOT / "scripts" / "detect-tools.sh"
+
+sys.path.insert(0, str(REPO_ROOT))
+
+BASH = shutil.which("bash") or ""
+pytestmark = pytest.mark.skipif(not BASH, reason="bash not on PATH")
+
+
+def _run_bash(script: str, tmp_path: Path) -> subprocess.CompletedProcess:
+    env = {
+        **os.environ,
+        "PROJECT_DIR": str(tmp_path),
+        "PIPELINE_DIR": str(REPO_ROOT),
+        "REMEMBER_DIR": str(tmp_path / ".remember"),
+    }
+    return subprocess.run(
+        [BASH, "-c", script],
+        capture_output=True,
+        text=True,
+        timeout=15,
+        env=env,
+    )
+
+
+# ── log.sh standalone — pins the canonical safe_eval ────────────────────
+
+def test_log_sh_safe_eval_strips_crlf(tmp_path):
+    """safe_eval must strip trailing \\r from CRLF input.
+
+    Without the strip, `EXCHANGE_COUNT=15\\r` fails integer tests in
+    save-session.sh on Windows.
+    """
+    script = f'source "{LOG_SH}"; safe_eval < <(printf "FOO=bar\\r\\n"); printf "[%s]" "$FOO"'
+    r = _run_bash(script, tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout == "[bar]", f"got {r.stdout!r}"
+
+
+def test_log_sh_safe_eval_crlf_value_is_integer(tmp_path):
+    """safe_eval value must be usable in `[ -eq ]` after CRLF input."""
+    script = (
+        f'source "{LOG_SH}"; '
+        'safe_eval < <(printf "NUM=42\\r\\n"); '
+        'if [ "$NUM" -eq 42 ]; then echo OK; else echo FAIL; fi'
+    )
+    r = _run_bash(script, tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout.strip() == "OK", f"got {r.stdout!r} / {r.stderr!r}"
+
+
+def test_log_sh_safe_eval_passes_backslash_path_verbatim(tmp_path):
+    """safe_eval stores Windows backslash paths verbatim (post-fix: emitter
+    no longer quotes, so safe_eval sees the raw path)."""
+    script = (
+        f'source "{LOG_SH}"; '
+        r"""safe_eval <<< "P=C:\Users\x.txt"; """
+        'printf "[%s]" "$P"'
+    )
+    r = _run_bash(script, tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout == r"[C:\Users\x.txt]", f"got {r.stdout!r}"
+
+
+# ── Real sourcing order — what orchestrators actually do ────────────────
+
+def test_real_sourcing_order_safe_eval_handles_crlf(tmp_path):
+    """detect-tools.sh THEN log.sh — the order used by save-session.sh."""
+    script = (
+        f'source "{DETECT_SH}"; source "{LOG_SH}"; '
+        'safe_eval < <(printf "FOO=bar\\r\\n"); '
+        'printf "[%s]" "$FOO"'
+    )
+    r = _run_bash(script, tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout == "[bar]", f"got {r.stdout!r}"
+
+
+# ── Python ↔ Bash roundtrip — the actual seam ───────────────────────────
+
+@pytest.mark.parametrize("value", [
+    "/tmp/remember-extract-abc.txt",                 # Linux temp path
+    r"C:\Users\VANDER~1\AppData\Local\Temp\x.txt",   # Windows temp path
+    "/path with spaces/file.txt",                    # spaces
+    "value-with-'-quote",                            # single quote
+    "plain",                                         # trivial
+])
+def test_shell_escape_safe_eval_roundtrip(value, tmp_path):
+    """`_shell_escape` output piped through `safe_eval` must roundtrip exactly.
+
+    This is the contract the bug violated: escape used POSIX
+    single-quote semantics, parser used verbatim semantics.
+    """
+    from pipeline.shell import _shell_escape
+
+    escaped = _shell_escape(value)
+    script = (
+        f'source "{LOG_SH}"; '
+        f'safe_eval <<< "VAL={escaped}"; '
+        'printf "%s" "$VAL"'
+    )
+    r = _run_bash(script, tmp_path)
+    assert r.returncode == 0, r.stderr
+    assert r.stdout == value, f"roundtrip broken: {r.stdout!r} != {value!r}"

tests/test_shell.py

@@ -31,15 +31,23 @@
 
 
 def test_shell_escape_simple():
-    assert _shell_escape("hello") == "'hello'"
+    # Post-#84: _shell_escape emits verbatim — safe_eval is verbatim too.
+    assert _shell_escape("hello") == "hello"
 
 
 def test_shell_escape_with_quotes():
-    assert _shell_escape("it's fine") == "'it'\\''s fine'"
+    # Single quotes pass through unchanged — safe_eval stores raw.
+    assert _shell_escape("it's fine") == "it's fine"
 
 
 def test_shell_escape_empty():
-    assert _shell_escape("") == "''"
+    assert _shell_escape("") == ""
+
+
+def test_shell_escape_rejects_newline():
+    import pytest as _pt
+    with _pt.raises(ValueError):
+        _shell_escape("has\nnewline")
 
 
 def test_cmd_parse_haiku_normal(capsys):