fix(hooks): portable mtime in macOS hook throttles; doc cleanup#1811
Conversation
Address review feedback surfaced on the 3.4.1 release promotion (#1810). Bug fix — `date -r FILE` is GNU-only. On BSD/macOS `date -r` expects epoch seconds, not a path, so the staleness/throttle checks in the new Cursor and Antigravity hooks silently failed on macOS: the state GC swept on every fire and the pending-save guard was skipped. Replace with a portable `os.path.getmtime` one-liner via the already-resolved $MEMPAL_PYTHON_BIN (cursor/lib, antigravity/lib, antigravity save hook). This restores the "bash 3.2.57 / macOS default" compatibility the Antigravity changelog claims. Docs: - Correct the MCP tool count to 33 (was 19/29/31 in 21 places across plugin manifests, READMEs, and website docs — all drifted from the TOOLS dict / mcp-tools.md reference, which both have 33). - Fix broken CHANGELOG link to the Cursor skill (skills/, not .cursor-plugin/skills/). - Fix one-too-many `../` in skills/mempalace/SKILL.md's cursor-hooks link (resolved above the repo root). - Add the required `mcpServers` wrapper to the mcp.json example in .cursor-plugin/README.md so copy-paste yields a valid Cursor config. Left intentionally unchanged: the os.dup2 fd-1 redirect in mcp_server.py is deliberate (#225 keeps JSON-RPC off fd 1).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates various documentation, plugin manifests, and metadata files to reflect the increase in the number of MCP tools to 33, and updates the Cursor plugin's MCP configuration structure. It also replaces the platform-dependent date -r command with a portable Python inline script to retrieve file modification times in the hook scripts. However, the reviewer identified a critical issue in the bash scripts where sys.argv[1] is used inside double quotes; the shell expands $1 as a positional parameter, leading to Python syntax errors. The reviewer provided suggestions to escape the dollar sign as \$1 to prevent this expansion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if [ -f "$marker" ]; then | ||
| local mtime now | ||
| if mtime=$(date -r "$marker" '+%s' 2>/dev/null) \ | ||
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$marker" 2>/dev/null) \ |
There was a problem hiding this comment.
In bash, double quotes allow parameter expansion. The expression sys.argv[1] contains $1, which is expanded by the shell to the first positional parameter of the current context (which is empty inside mempal_gc_stale_state). This results in a Python syntax error (sys.argv[]) and causes the check to silently fail. Escape the $ as \$1 to pass it literally to Python.
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$marker" 2>/dev/null) \ | |
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[\$1])))" "$marker" 2>/dev/null) \ |
| # mtime in epoch seconds (date -r); if stale (> 1 hour), reclaim. | ||
| if mtime=$(date -r "$PENDING_FILE" '+%s' 2>/dev/null) \ | ||
| # mtime in epoch seconds (portable; BSD/macOS `date -r` takes epoch, not a path). | ||
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$PENDING_FILE" 2>/dev/null) \ |
There was a problem hiding this comment.
In bash, double quotes allow parameter expansion. The expression sys.argv[1] contains $1, which is expanded by the shell to the first positional parameter of the script. This results in a Python syntax error or incorrect behavior. Escape the $ as \$1 to pass it literally to Python.
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$PENDING_FILE" 2>/dev/null) \ | |
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[\$1])))" "$PENDING_FILE" 2>/dev/null) \ |
| if [ -f "$marker" ]; then | ||
| local mtime now | ||
| if mtime=$(date -r "$marker" '+%s' 2>/dev/null) \ | ||
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$marker" 2>/dev/null) \ |
There was a problem hiding this comment.
In bash, double quotes allow parameter expansion. The expression sys.argv[1] contains $1, which is expanded by the shell to the first positional parameter of the current context (which is empty inside mempal_gc_stale_state). This results in a Python syntax error (sys.argv[]) and causes the check to silently fail. Escape the $ as \$1 to pass it literally to Python.
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[1])))" "$marker" 2>/dev/null) \ | |
| if mtime=$("$MEMPAL_PYTHON_BIN" -c "import os, sys; print(int(os.path.getmtime(sys.argv[\$1])))" "$marker" 2>/dev/null) \ |
There was a problem hiding this comment.
Pull request overview
This PR fixes macOS/BSD compatibility in the Cursor and Antigravity hook throttle/guard logic by replacing GNU-specific date -r FILE usage with a portable mtime lookup via Python, and then normalizes documentation/tooling claims across the repo to consistently reflect the current MCP tool count.
Changes:
- Replace
date -r <file> '+%s'withpython -c "os.path.getmtime(...)"in hook staleness/throttle checks for macOS portability. - Update docs, plugin manifests, and READMEs to consistently claim 33 MCP tools (matching
mempalace/mcp_server.py’sTOOLSdict). - Fix broken/incorrect relative links (notably the Cursor skill link in
CHANGELOG.mdand a relative website link inskills/mempalace/SKILL.md).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| website/reference/modules.md | Updates MCP server tool-count claim to 33 in module overview text. |
| website/reference/mcp-tools.md | Updates top-level statement to “33 MCP tools”. |
| website/guide/openclaw.md | Updates OpenClaw integration doc to reflect 33 tools. |
| website/guide/mcp-integration.md | Updates MCP integration guide references from 29 → 33 tools. |
| website/guide/claude-code.md | Updates Claude Code guide bullet to 33 tools. |
| skills/mempalace/SKILL.md | Updates Cursor-specific tool-count claim and fixes relative link to website guide. |
| README.md | Updates project README MCP tool-count claim to 33. |
| mempalace/README.md | Updates package README module table entry to 33 tools. |
| hooks/cursor/lib/common.sh | Uses portable Python getmtime() for daily GC throttle marker instead of date -r. |
| hooks/antigravity/mempal_save_hook_antigravity.sh | Uses portable Python getmtime() for pending-save marker staleness guard. |
| hooks/antigravity/lib/common.sh | Uses portable Python getmtime() for daily GC throttle marker instead of date -r. |
| CHANGELOG.md | Fixes Cursor skill link target to skills/mempalace/SKILL.md. |
| .cursor-plugin/README.md | Updates tool count claims and corrects the mcp.json example shape (mcpServers wrapper). |
| .cursor-plugin/plugin.json | Updates description tool-count claim to 33. |
| .cursor-plugin/marketplace.json | Updates marketplace description tool-count claim to 33. |
| .codex-plugin/README.md | Updates tool-count claim to 33. |
| .codex-plugin/plugin.json | Updates description fields to claim 33 tools. |
| .claude-plugin/README.md | Updates tool-count claim to 33. |
| .claude-plugin/plugin.json | Updates description tool-count claim to 33. |
| .claude-plugin/marketplace.json | Updates marketplace description tool-count claim to 33. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The snippet has no shell interpolation — the path arrives via argv, not string interpolation — so single quotes are correct and make it unambiguous that nothing is shell-expanded. Behavior is identical: `sys.argv[1]` contains no `$`, so it was never expanded (verified empirically). Matches the single-quoted `python -c` blocks already in hooks/cursor/lib/common.sh. No functional change.
Addresses the bot review feedback raised on the 3.4.1 release promotion (#1810). All items verified against the code, not applied blindly.
Bug fix —
date -rbreaks on macOS (Gemini, 3 hits)date -r FILEis GNU coreutils only. On BSD/macOS,date -rinterprets its argument as epoch seconds, not a file path, so it errors on the path string. With2>/dev/nullin an&&chain, the staleness/throttle checks silently evaluate false:Fixed with a portable
os.path.getmtimeone-liner via the already-resolved$MEMPAL_PYTHON_BIN. This is in the headline Cursor/Antigravity hooks shipping in 3.4.1, and restores the "bash 3.2.57 / macOS default" compatibility the changelog claims.Docs (Copilot + Gemini)
website/reference/mcp-tools.md###headings, both test-enforced) is 33. Updated all to agree.skills/mempalace/SKILL.md, not.cursor-plugin/skills/...(that dir has noskills/).../../../website/...resolved above the repo root fromskills/mempalace/; corrected to../../.mcp.jsonexample — added themcpServerswrapper that the real shippedmcp.jsonuses, so copy-paste yields a valid Cursor config.Verified NOT a bug — left unchanged
Gemini's HIGH on
mcp_server.pyos.dup2(fd 1)is deliberate and documented (#225 keeps the JSON-RPC channel off fd 1; the dup captures C-level onnxruntime/chromadb banners). Changing it risks regressing #225.Note on the drift treadmill
The README tool-count test (
test_readme_claims.py) uses(\d+)\s+tools, which misses the"N MCP tools"phrasing (the "MCP" between number and "tools"), so these counts can drift unnoticed. Worth tightening the guard in a follow-up; out of scope here.Validation
bash -nclean on all 3 hook scriptsruff check+ruff format --check— cleanMerging this into
developflows it into the #1810 release promotion.