fix(antigravity): address gemini-code-assist review on PR #1633

Five fixes for issues called out by the gemini-code-assist[bot] review.
Each gets a regression test that locks in the correction.

1. CRITICAL: marker-cleanup watcher used POSIX `wait` on a sibling pid
   (save hook). bash `wait` only works on direct children of the
   calling shell — the `( wait $MINE_PID ... ) &` subshell runs as a
   sibling of MINE_PID, so wait fails immediately and the pending
   marker is deleted within milliseconds, defeating the concurrency
   guard. Replace with `while kill -0 $MINE_PID; do sleep 1; done`,
   which queries pid existence regardless of parent-child relationship.
   Test: test_save_hook_marker_watcher_uses_kill_polling.

2. Bare `mempalace` console-script invocation in the save hook fails
   when the venv's bin/ is not on the hook's PATH (e.g. uv tool
   install in some configurations, manually managed virtualenvs).
   Switch to `"$MEMPAL_PYTHON_BIN" -m mempalace mine ...` so the
   resolved interpreter runs the package directly via
   mempalace/__main__.py. Tests:
   test_save_hook_uses_python_module_invocation,
   test_save_hook_missing_mempalace_python_module_does_not_crash.

3. Same issue in the wake hook's inner Python helper. Switch
   `['mempalace', 'wake-up', ...]` to `[sys.executable, '-m',
   'mempalace', 'wake-up', ...]` — sys.executable is the same
   interpreter that resolved MEMPAL_PYTHON in lib/common.sh.
   Test: test_wake_hook_uses_sys_executable_module_invocation.

4. The Python parser in lib/common.sh wrapped `json.load` in
   `try/except` and silently fell back to `data = {}`. The script
   then printed the `__MEMPAL_PARSE_OK__` sentinel even on parse
   failure, so the bash sentinel-check on the caller side
   (`[ "$_marker" != "__MEMPAL_PARSE_OK__" ]`) never triggered the
   defense-in-depth `input parse failed` branch. Remove the
   try/except so the exception propagates, Python exits non-zero,
   and the sentinel is omitted on bad JSON. The traceback still
   lands in antigravity_last_python_err.log for debugging.
   Test: test_common_sh_parser_omits_sentinel_on_malformed_json.

5. `mempal_save_interval()` failed to strip leading zeros from
   MEMPAL_SAVE_INTERVAL. Values like "08" or "09" then crashed the
   modulo step `$((COUNT % INTERVAL))` because bash arithmetic
   parses tokens starting with `0` as octal, and 8/9 are not valid
   octal digits ("value too great for base"). Strip leading zeros
   while preserving the literal "0" (which is then floored to 15).
   Test: test_save_hook_handles_leading_zero_save_interval (4 cases).

Plus one cosmetic fix in install.sh: removed a no-op `(cd "$OLDPWD"
2>/dev/null || cd .) >/dev/null 2>&1` line in mempal_absolutize().
The subshell cd doesn't affect the parent shell, and the installer
never cd's in the main shell anyway, so $PWD is already correct.

Verification:
* 9 new regression tests, all 65 antigravity tests pass
* full repo: 2323 passed (was 2314), 3 skipped, 1 unrelated warning
* ruff check + ruff format --check both clean across 139 files
* bash -n clean on all four shell files
* clean reinstall to ~/.gemini/config/plugins/mempalace/ succeeds
* idempotent re-run produces zero file writes (cmp-gated)
* both hooks return {} exit 0 with synthetic camelCase stdin

Co-authored-by: Cursor <cursoragent@cursor.com>

hooks/antigravity/install.sh

@@ -121,9 +121,10 @@ mempal_absolutize() {
         /*) printf '%s' "$p" ;;
         ~*) printf '%s' "${p/#\~/$HOME}" ;;
         *)
-            # Resolve relative to the user's $PWD at invocation time, not
-            # the repo root.
-            (cd "$OLDPWD" 2>/dev/null || cd .) >/dev/null 2>&1
+            # Resolve relative to the user's $PWD at invocation time.
+            # We never `cd` in the main shell of this installer, so
+            # $PWD is already the user's invocation directory — no
+            # subshell cd dance needed.
             local base="${PWD}"
             printf '%s/%s' "$base" "$p"
             ;;

hooks/antigravity/lib/common.sh

@@ -166,10 +166,15 @@ mempal_parse_stdin() {
         printf '%s' "$input" | "$MEMPAL_PYTHON_BIN" -c "
 import sys, json, re
 
-try:
-    data = json.load(sys.stdin)
-except Exception:
-    data = {}
+# IMPORTANT: do NOT wrap json.load in a try/except. If the input is
+# not valid JSON we want Python to exit non-zero BEFORE printing the
+# __MEMPAL_PARSE_OK__ sentinel — the bash caller looks for the
+# sentinel on line 1 to decide whether to engage its defense-in-depth
+# 'failed to parse' branch. Catching the exception and falling back
+# to data={} would let the sentinel print, masking parse failures
+# from the bash side. The traceback lands in
+# antigravity_last_python_err.log so operators can debug.
+data = json.load(sys.stdin)
 
 def safe(s, allowed=r'[^a-zA-Z0-9_/.\-~]'):
     return re.sub(allowed, '', str(s))
@@ -285,6 +290,14 @@ mempal_save_interval() {
     case "$raw" in
         ''|*[!0-9]*) printf '15'; return 0 ;;
     esac
+    # Strip leading zeros. bash arithmetic ($((...))) parses any token
+    # starting with `0` as octal, so MEMPAL_SAVE_INTERVAL=08 would
+    # crash $((COUNT % INTERVAL)) with "value too great for base".
+    # Loop while the value still starts with 0 AND has length > 1, so
+    # the literal string "0" is preserved (then floored to 15 below).
+    while [ "${raw}" != "${raw#0}" ] && [ "${#raw}" -gt 1 ]; do
+        raw="${raw#0}"
+    done
     if [ "$raw" -lt 1 ] 2>/dev/null; then
         printf '15'
         return 0

hooks/antigravity/mempal_save_hook_antigravity.sh

@@ -191,8 +191,16 @@ mempal_log "stop" "$CONVERSATION_ID" "TRIGGERING SAVE wing=$WING transcript_dir=
 # sufficient; the parent (this hook script) can exit and the child
 # reparents to init. Stdout and stderr both go to the antigravity hook
 # log so a wedged mine surfaces in one place.
-if command -v mempalace >/dev/null 2>&1; then
-    nohup mempalace mine "$TRANSCRIPT_DIR" \
+#
+# We invoke mempalace as `"$MEMPAL_PYTHON_BIN" -m mempalace` rather than
+# the bare `mempalace` console script so a user with the package
+# installed only inside a venv (and the venv's bin/ not on the hook's
+# PATH, e.g. `uv tool install` in some distributions, or a manually
+# managed virtualenv) still hits a working mine. MEMPAL_PYTHON honours
+# user override; sees ``mempalace/__main__.py`` which dispatches to
+# ``mempalace.cli:main`` — identical to the console script.
+if "$MEMPAL_PYTHON_BIN" -m mempalace --version >/dev/null 2>&1; then
+    nohup "$MEMPAL_PYTHON_BIN" -m mempalace mine "$TRANSCRIPT_DIR" \
         --mode convos \
         --wing "$WING" \
         >> "$MEMPAL_AGY_LOG" 2>&1 < /dev/null &
@@ -201,14 +209,29 @@ if command -v mempalace >/dev/null 2>&1; then
     mempal_log "stop" "$CONVERSATION_ID" "mine spawned pid=$MINE_PID wing=$WING"
 
     # Schedule a marker-cleanup detach so the marker doesn't outlive a
-    # crashed mine. We can't `wait` because that would block the hook;
-    # instead, fire-and-forget a tiny watcher.
+    # crashed mine. We can't `wait` here because:
+    #   (1) bash `wait` only operates on direct children of the
+    #       calling shell; the subshell `( ... ) &` below runs as a
+    #       SIBLING of MINE_PID, not its parent, so `wait $MINE_PID`
+    #       fails IMMEDIATELY with "not a child of this shell" and
+    #       the marker would be deleted within milliseconds — even
+    #       while the mine is still running.
+    #   (2) We can't use `wait` directly in the parent either,
+    #       because that would block the hook for the full mine
+    #       runtime and Antigravity would hang waiting for stdout.
+    # The portable fix is `kill -0 $pid` polling: signal 0 doesn't
+    # actually deliver a signal, it just queries whether the pid is
+    # alive (regardless of parent-child relationship). The inner
+    # subshell is detached so the hook returns immediately, and the
+    # marker is removed only AFTER the mine actually exits.
     (
-        wait "$MINE_PID" 2>/dev/null
+        while kill -0 "$MINE_PID" 2>/dev/null; do
+            sleep 1
+        done
         rm -f "$PENDING_FILE" 2>/dev/null
     ) >/dev/null 2>&1 < /dev/null &
 else
-    mempal_log "stop" "$CONVERSATION_ID" "ERROR: mempalace CLI not on PATH; install or set MEMPAL_PYTHON"
+    mempal_log "stop" "$CONVERSATION_ID" "ERROR: mempalace is not runnable via $MEMPAL_PYTHON_BIN -m mempalace; install mempalace or set MEMPAL_PYTHON"
     rm -f "$PENDING_FILE" 2>/dev/null
 fi
 

hooks/antigravity/mempal_wake_hook_antigravity.sh

@@ -115,9 +115,14 @@ import json, subprocess, sys
 wing = sys.argv[1]
 timeout_s = 0.5  # 500 ms
 
+# Invoke as ``[sys.executable, '-m', 'mempalace', ...]`` rather than
+# the bare ``mempalace`` console script. sys.executable is the same
+# Python that resolved MEMPAL_PYTHON in lib/common.sh, so this binds
+# the wake-up call to the correct interpreter (and its installed
+# mempalace package) even when the venv's bin/ isn't on PATH.
 try:
     completed = subprocess.run(
-        ['mempalace', 'wake-up', '--wing', wing],
+        [sys.executable, '-m', 'mempalace', 'wake-up', '--wing', wing],
         capture_output=True,
         text=True,
         timeout=timeout_s,

tests/test_antigravity_hooks_shell.py

@@ -321,6 +321,190 @@ def test_save_hook_floors_negative_save_interval(tmp_path: Path) -> None:
     assert result.stdout.strip() == "{}"
 
 
+@pytest.mark.parametrize("interval", ["08", "09", "008", "0099"])
+def test_save_hook_handles_leading_zero_save_interval(tmp_path: Path, interval: str) -> None:
+    """MEMPAL_SAVE_INTERVAL with leading zeros must NOT trigger bash octal arithmetic.
+
+    bash arithmetic ($((COUNT % INTERVAL))) parses any token starting
+    with `0` as octal. Values like "08" or "09" are not valid octal
+    digits and would crash the modulo step with::
+
+        bash: 08: value too great for base (error token is "08")
+
+    mempal_save_interval() in lib/common.sh strips leading zeros before
+    returning. Regression test for gemini-code-assist review on PR
+    #1633.
+    """
+    state = tmp_path / "state"
+    home = tmp_path / "home"
+    _ensure_palace(home)
+    result = _run_hook(
+        SAVE_HOOK,
+        _stop_payload(),
+        state_dir=state,
+        home=home,
+        extra_env={"MEMPAL_SAVE_INTERVAL": interval},
+    )
+    assert result.returncode == 0, (
+        f"save hook crashed on MEMPAL_SAVE_INTERVAL={interval!r}:\n"
+        f"stdout={result.stdout!r}\nstderr={result.stderr!r}"
+    )
+    assert result.stdout.strip() == "{}"
+    # Stderr must not contain the octal "value too great for base" error.
+    assert "value too great for base" not in result.stderr, (
+        f"bash octal parse error leaked through for MEMPAL_SAVE_INTERVAL={interval!r}: "
+        f"{result.stderr!r}"
+    )
+
+
+def test_common_sh_parser_omits_sentinel_on_malformed_json(tmp_path: Path) -> None:
+    """`mempal_parse_stdin` must NOT print the success sentinel on parse failure.
+
+    The bash callers detect parse failure by checking whether line 1
+    of the parser output is exactly ``__MEMPAL_PARSE_OK__``. If
+    json.load is wrapped in try/except (and falls back to data={}),
+    the sentinel still gets printed and the bash defense-in-depth
+    branch never engages. Regression test for gemini-code-assist
+    review on PR #1633.
+    """
+    state = tmp_path / "state"
+    home = tmp_path / "home"
+    _ensure_palace(home)
+    state.mkdir(parents=True, exist_ok=True)
+    # Source the lib and call mempal_parse_stdin with malformed JSON.
+    cmd = f". {COMMON_LIB}; mempal_parse_stdin '{{not even close to json{{'"
+    result = subprocess.run(
+        ["bash", "-c", cmd],
+        capture_output=True,
+        text=True,
+        env={
+            **os.environ,
+            "HOME": str(home),
+            "MEMPAL_STATE_DIR": str(state),
+        },
+        timeout=10,
+    )
+    # The function itself shouldn't error (the inner Python crashes,
+    # but the subshell catches it). Stdout must NOT contain the sentinel.
+    assert "__MEMPAL_PARSE_OK__" not in result.stdout, (
+        f"parser printed success sentinel on bad JSON, defeating "
+        f"bash-side error detection: stdout={result.stdout!r}"
+    )
+
+
+def test_save_hook_missing_mempalace_python_module_does_not_crash(tmp_path: Path) -> None:
+    """When the resolved Python interpreter cannot run `-m mempalace`, fail open.
+
+    The save hook now invokes mempalace via ``"$MEMPAL_PYTHON_BIN"
+    -m mempalace mine ...`` rather than the bare ``mempalace`` console
+    script. If MEMPAL_PYTHON points at an interpreter that doesn't
+    have the package installed, the hook must log the failure and
+    still emit ``{}`` — never crash, never block the user.
+    """
+    state = tmp_path / "state"
+    home = tmp_path / "home"
+    _ensure_palace(home)
+    transcript = tmp_path / "transcript.jsonl"
+    transcript.write_text("{}\n", encoding="utf-8")
+    # Point MEMPAL_PYTHON at a stub interpreter that has no mempalace
+    # package installed — `python -m mempalace --version` will fail.
+    stub = tmp_path / "stub_python"
+    stub.write_text(
+        "#!/bin/sh\n"
+        "# Minimal python stub: rejects every -m invocation so the\n"
+        '# hook hits the "module unrunnable" branch.\n'
+        'case "$*" in\n'
+        '    *"-m mempalace"*) exit 1 ;;\n'
+        '    *) exec /usr/bin/env python3 "$@" ;;\n'
+        "esac\n",
+        encoding="utf-8",
+    )
+    stub.chmod(0o755)
+    result = _run_hook(
+        SAVE_HOOK,
+        _stop_payload(transcriptPath=str(transcript)),
+        state_dir=state,
+        home=home,
+        extra_env={
+            "MEMPAL_PYTHON": str(stub),
+            "MEMPAL_SAVE_INTERVAL": "1",
+        },
+    )
+    assert result.returncode == 0, result.stderr
+    assert result.stdout.strip() == "{}"
+    log_body = (state / "antigravity_hook.log").read_text(errors="replace")
+    assert "is not runnable via" in log_body, (
+        f"expected the new 'mempalace not runnable via $MEMPAL_PYTHON_BIN' log "
+        f"line; got:\n{log_body}"
+    )
+
+
+def test_save_hook_uses_python_module_invocation(tmp_path: Path) -> None:
+    """The save hook source MUST invoke mempalace via `-m mempalace`.
+
+    Locks in the gemini-code-assist fix so a future edit doesn't
+    silently regress to the bare ``mempalace`` console-script call,
+    which fails when the user's PATH doesn't expose the venv bin.
+    """
+    body = SAVE_HOOK.read_text(encoding="utf-8")
+    assert '"$MEMPAL_PYTHON_BIN" -m mempalace' in body, (
+        "save hook should invoke mempalace via $MEMPAL_PYTHON_BIN -m mempalace, "
+        "not the bare `mempalace` console script. The bare invocation breaks "
+        "when the venv's bin/ isn't on the hook's PATH."
+    )
+    # Also verify the bare invocation is gone (defense-in-depth).
+    # Allow `mempalace` to appear in comments / strings, but not as
+    # the start of a `nohup ... mempalace mine` command.
+    assert "nohup mempalace " not in body, (
+        "bare `nohup mempalace ...` invocation found; should be "
+        '`nohup "$MEMPAL_PYTHON_BIN" -m mempalace ...`'
+    )
+
+
+def test_save_hook_marker_watcher_uses_kill_polling(tmp_path: Path) -> None:
+    """The marker-cleanup watcher must use `kill -0` polling, not `wait`.
+
+    bash `wait` only operates on direct children of the calling
+    shell. The watcher subshell `( ... ) &` runs as a SIBLING of the
+    mine pid, not its parent, so `wait $MINE_PID` fails immediately
+    with "not a child of this shell" and the marker would be deleted
+    within milliseconds — even while the mine is still running. The
+    correct primitive is `kill -0 $pid` which queries existence
+    regardless of parent-child relationship. Regression test for
+    gemini-code-assist review on PR #1633.
+    """
+    body = SAVE_HOOK.read_text(encoding="utf-8")
+    assert 'kill -0 "$MINE_PID"' in body, (
+        "marker-cleanup watcher must poll with `kill -0 $MINE_PID`, not `wait`. "
+        "`wait` only operates on direct children; the sibling subshell would "
+        "error out and delete the marker prematurely."
+    )
+    # Defense-in-depth: ensure the buggy `wait "$MINE_PID"` is gone.
+    assert 'wait "$MINE_PID"' not in body, (
+        'buggy `wait "$MINE_PID"` still present in the watcher subshell. '
+        "POSIX wait cannot watch a sibling pid."
+    )
+
+
+def test_wake_hook_uses_sys_executable_module_invocation(tmp_path: Path) -> None:
+    """The wake hook's inner Python must invoke mempalace via sys.executable -m.
+
+    Same rationale as the save hook fix: the bare ``mempalace``
+    console script fails when the venv's bin/ isn't on the hook's
+    PATH. Using ``[sys.executable, '-m', 'mempalace', ...]`` binds
+    the call to the same interpreter that resolved MEMPAL_PYTHON.
+    """
+    body = WAKE_HOOK.read_text(encoding="utf-8")
+    assert "sys.executable, '-m', 'mempalace'" in body, (
+        "wake hook should invoke mempalace via [sys.executable, '-m', 'mempalace', ...], "
+        "not ['mempalace', ...]. The bare invocation breaks when the venv's bin/ "
+        "isn't on the hook's PATH."
+    )
+    assert "['mempalace', 'wake-up'" not in body, (
+        "bare ['mempalace', 'wake-up', ...] invocation found in wake hook"
+    )
+
+
 def test_save_hook_rejects_traversal_in_transcript_path(tmp_path: Path) -> None:
     """A `..` segment in transcriptPath must be rejected."""
     state = tmp_path / "state"