A daemon job refused the palace write lock was marked terminal `failed` and
never retried. `mine_palace_lock` guards the palace write itself, so a refusal
means no drawer was filed, but the queue recorded it exactly like a crash
mid-execution, whose outcome is unknown. The work was dropped: it only ran
again if a hook happened to re-submit equivalent work, and a job kind no hook
re-emits was lost outright.
Refusals now defer. The job goes back to `queued` with `started_at` cleared,
the claim's attempt increment undone, and the reason recorded. The update is
scoped to the claim that was refused (started_at match), so a defer racing a
recovery re-claim cannot re-queue work it does not own. `claim_next` clears
the recorded reason so it never outlives the claim it describes. Every other
failure stays terminal: a crashed job's outcome is unknown, and blindly
re-running a non-idempotent kind would re-file verbatim content, which is what
MAX_ATTEMPTS guards.
The worker cools a refused job off in memory and moves on rather than sleeping
in line. It is the only worker and the holder keeps the lock until its write
finishes, which can be a long mine, so blocking would stall every unrelated job
behind a lock that has nothing to do with them, and a job merely queued behind
the refused one would never be claimed. `claim_next(exclude=...)` skips a
cooling job, so the oldest-first ordering cannot hand the same refused job back
forever while newer work waits. The filter runs in Python rather than an
`id NOT IN (?, ?, ...)` list, which would bind one host parameter per cooling
job against a cap that defaults to 999 before SQLite 3.32. The cooldown
lives in the worker, not the schema, which has no migrations; a restart just
retries at once, costing one refusal, never work.
`tool_diary_write` swallowed `MineAlreadyRunning` in its bare `except
Exception`, so the refusal reached the daemon with no `error_class` and
`diary_write` would still have been dead-lettered. It now uses a typed handler
ahead of the bare `Exception`, the way `tool_mine` and `tool_sync` already do.
Deferral makes a refused job non-terminal, and `DaemonClient.wait` only returns
on a terminal state, so callers that wait on purpose would have waited for a
state a parked job cannot reach: a foreground `mine --daemon` for the one-hour
default, and the `hooks_cli` pre-compaction mine and Stop-hook diary paths for
their whole timeout on every fire, each then reporting a failure that never
happened. `wait(stop_on_lock_deferral=True)` hands the parked job back instead.
The CLI echoes the global `--palace` back into the command it suggests, so the
suggestion does not silently list the default palace's queue instead of the
one the job is parked in. A job that is genuinely running is still waited out.
Co-Authored-By: mjvmsteixeira <185609735+mjvmsteixeira@users.noreply.github.com>
* fix: sanitize wing slug for project dirs with special characters
Project folders containing characters outside sanitize_name's set
(e.g. a leading '+') leaked into the derived wing name, producing names
like 'wing_+project' that config.sanitize_name rejects, silently
breaking diary auto-save for that project.
Add _safe_wing_slug(): collapse non-word runs to '_', trim, and fall
back to 'sessions' when a name reduces to nothing. Route the three
wing-derivation sites through it.
Tests: unit cases for the helper plus a hypothesis property test
asserting wing_<slug> always passes sanitize_name for any input.
* fix: preserve dots and apostrophes in wing slug for backward compatibility
The first pass collapsed every non-word character (including dot and
apostrophe) to underscore, renaming existing valid wings — e.g. my.app
became wing_my_app — which would orphan diary entries already filed under
the old name. Keep dot and apostrophe (both accepted by sanitize_name),
collapse consecutive dots to avoid the path-traversal rejection, and trim
edge separators.
Add backward-compatibility tests for previously-valid names plus a
double-dot collapse test.
* fix: cap wing slug length to stay within sanitize_name's limit
sanitize_name rejects names over 128 characters, so a very long project
directory name would produce a wing name that fails validation,
re-triggering the silent auto-save break this PR fixes. Truncate the slug
to 120 chars (the wing_ prefix keeps the total under 128).
Widen the hypothesis property test to max_size=300 so it exercises the
length path, and add an explicit truncation test.
Addresses gemini-code-assist review feedback on PR #1852.
---------
Co-authored-by: Ivan Antsimonau <ivan.antsimonau@katim.com>
Short sessions that exit cleanly below SAVE_INTERVAL and without a PreCompact
were never saved. Add a SessionEnd hook that takes one final flush.
Claude Code budgets SessionEnd hooks at 1.5s and a plugin-provided timeout
cannot raise it, and a cold mempalace start exceeds that, so the wrapper
backgrounds the work and returns immediately; the detached child completes the
transcript ingest, project mine, and diary checkpoint after the session exits.
The handler validates transcript_path through _validate_transcript_path before
any ingest or diary write, so a traversal or wrong-suffix path is rejected while
the independent project mine still runs.
Adds hook_session_end, both shell wrappers, the plugin hooks.json entry, the
session-end CLI choice, and focused tests.
(cherry picked from commit 10e1450e04fc7cec72984ab7442d3b4fca1490e8)
- New mempalace/daemon.py: long-lived localhost HTTP server (127.0.0.1) with a
SQLite WAL job queue, single worker thread, bearer-token auth, and owner-only
file perms (0600/0700) on queue DB, token, endpoint, and log.
- New mempalace/service.py: transport-neutral job execution surface shared by the
daemon, with per-job env isolation so one job's backend/palace switch cannot
leak into the next. mcp_tool is allowlisted to write-classified tools only.
- Crash recovery re-queues jobs left 'running' by a killed daemon; jobs that
already exhausted MAX_ATTEMPTS are dead-lettered to 'failed' instead of being
retried (non-idempotent diary_write would otherwise duplicate verbatim
content on every restart).
- Bounded retention prunes terminal jobs older than 7 days
(MEMPALACE_DAEMON_RETENTION_DAYS); queued/running jobs are never touched so a
crash mid-prune cannot drop in-flight work.
- CLI: --daemon/--background on mine/sync submit to the queue; new
`mempalace daemon {start,stop,status,jobs,wait}` subcommand. Strictly opt-in:
no flag, env, or config means no daemon and no behavior change.
- Hooks opt in via MEMPALACE_HOOKS_DAEMON or config hooks.daemon; when the daemon
is not already running, hooks fall back to the existing direct/spawn path so
the 500ms hook budget is preserved (hooks never auto-start the daemon).
- service.run_sync renders the same operator-facing report shape as the direct
CLI sync path (no_source, out_of_scope, by_source, Re-run/Removed hints) and
drops the old KeyError-prone 'deleted' read.
Stop-hook checkpoints were saved via _save_diary_direct with a hardcoded
agent_name="session-hook". tool_diary_read filters Chroma metadata by
agent, so mempalace_diary_read(agent_name="claude") never surfaced any
hook-saved checkpoint. Derive the diary identity from the harness
(claude-code -> claude, codex -> codex; an unknown harness keeps its own
name) and thread it through to tool_diary_write. Make agent_name a
required keyword argument so the identity is always explicit.
Co-Authored-By: YC-AIUSER <273917354+YC-AIUSER@users.noreply.github.com>
test_precompact_hook_enabled_by_default asserted
result["decision"] == "block", but hook_precompact has never emitted
decision — it mines synchronously and returns {}. Assertion was
copy-pasted from the stop-hook test. Fix to assert result == {} with
_mine_sync mocked so the test verifies the real contract (enabled →
mine + pass through) without actually mining.
Plus ruff format on 6 test files the CI pin flagged.
- Add parenthetical hints to block reasons so AI knows what each tool
saves (session summary, quotes/decisions/code)
- Remove dead config file creation from test_stop_hook_disabled_by_config
- Add missing test for MEMPALACE_HOOKS_AUTO_SAVE=no env var
- Replace bare except: with except Exception: in shell scripts
Add a clean opt-out for auto-save hook blocking (closes#494).
- New `hooks.auto_save` config option (default: true) in
~/.mempalace/config.json and MEMPALACE_HOOKS_AUTO_SAVE env var
- When disabled, stop and precompact hooks pass through without blocking
- Shorten block reason text from 6-line instructions to single-line
prompts — reduces UI noise while keeping tool names explicit
- Both Python (hooks_cli.py) and standalone shell scripts respect the
toggle via config file or env var
Two medium-priority gemini-code-assist comments on PR #1580 both
recommend more-idiomatic Python:
1. **Production code (``mempalace/hooks_cli.py::_mempalace_python``)** —
replace ``try/except IndexError`` with ``if len(parents) > N:``
look-before-you-leap checks. Exception handling for bounded-integer
index lookups is a code smell in Python; LBYL makes the depth check
explicit and removes exception overhead. Same behavior, clearer
intent.
Before (EAFP, ~12 lines + comment):
try:
venv_bin = resolved.parents[3] / "bin" / "python"
if venv_bin.is_file():
return str(venv_bin)
except IndexError:
pass
After (LBYL, ~5 lines):
if len(parents) > 3:
venv_bin = parents[3] / "bin" / "python"
if venv_bin.is_file():
return str(venv_bin)
2. **Test code (``tests/test_hooks_cli.py``)** — replace the lambda +
generator-throw hack with ``MagicMock.side_effect = get_item``,
where ``get_item`` is a normal function that returns the
editable-install path for index 1 and raises ``IndexError`` for
any other index (defensive against a future regression that drops
the LBYL length check). Standard ``side_effect`` mocking pattern.
Before:
fake_parents.__getitem__ = lambda self, idx: (
RealPath("/work/mempalace")
if idx == 1
else (_ for _ in ()).throw(IndexError(idx))
)
After:
def get_item(idx):
if idx == 1:
return RealPath("/work/mempalace")
raise IndexError(idx)
fake_parents.__len__.return_value = 3
fake_parents.__getitem__.side_effect = get_item
Also added ``__len__`` mock so the LBYL length check in production
sees the simulated shallow path correctly.
## Verification
pytest tests/test_hooks_cli.py
→ 110 passed, 1 skipped (same as PR #1580 baseline; regression
test for shallow-path crash still GREEN)
ruff check + ruff format --check (pinned 0.15.9)
→ All checks passed; 2 files already formatted
``_mempalace_python()`` in ``mempalace/hooks_cli.py`` uses
``Path(__file__).resolve().parents[3]`` to locate the venv Python
interpreter in the standard install layout
``<venv>/lib/pythonX.Y/site-packages/mempalace/hooks_cli.py``. When the
package lives at a shallow filesystem path — Docker containers
mounting at ``/work``, ``/opt/app``, minimal-prefix production
installs — ``parents`` has fewer than 4 elements and the index raises
``IndexError`` instead of falling through to the editable-install
branch.
The crash was caught by OrbStack-based triple-Python CI verification
on PR #1579: 16 tests in ``test_hooks_cli.py`` failed identically on
Linux 3.9 / 3.11 / 3.13 with the same ``IndexError: 3`` from
``pathlib._PathBase.parents.__getitem__`` — and verified pre-existing
on develop tip in the same container. The bug never surfaces in
GitHub Actions CI runners (their workdir at
``/home/runner/work/mempalace/mempalace`` has plenty of parent
directories) but it surfaces immediately for anyone:
- running mempalace in editable mode inside a Docker dev container
- shipping mempalace as part of an OCI image where the install
prefix is ``/app`` or ``/opt/<name>``
- using OrbStack / Colima / podman-machine for cross-version
verification
## The fix
Wrap each ``parents[N]`` access in ``try/except IndexError`` so the
helper falls through to the next strategy (editable-install →
``sys.executable``) instead of crashing the hook. Both ``parents[3]``
AND ``parents[1]`` are guarded — the latter is defensive against
extreme cases like a file at root (``/file.py``, parents=[/]) — same
class of bug.
## Test added (RED-first, then GREEN)
tests/test_hooks_cli.py::test_mempalace_python_handles_shallow_path_without_crashing
Mocks ``Path(__file__).resolve()`` so ``parents[3]`` raises
``IndexError`` and ``parents[1]`` returns a real shallow path
(``/work/mempalace``). Pre-commit: function raises ``IndexError: 3``.
Post-commit: function returns a valid Python interpreter path
(either editable-venv if present, otherwise ``sys.executable``).
## Verification
pytest tests/test_hooks_cli.py
→ 110 passed, 1 skipped on macOS (the existing run)
→ 110 passed, 1 skipped on Linux 3.9 / 3.11 / 3.13 (OrbStack)
— was 16 failed, 94 passed before this commit
ruff check + ruff format --check (pinned 0.15.9)
→ All checks passed; 2 files already formatted
- Disable mine and MCP idle timeouts when env values are invalid.
- Use bare-PID slot file mtime as the compatibility timestamp instead of treating old slots as infinitely stale.
- Treat malformed PID-slot timestamps as stale without crashing hook execution.
- Use process-level idle watchdog termination so stale MCP servers actually release file handles.
- Restore tenant-isolation assertions accidentally removed from the KG cache test.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add MEMPALACE_MINE_TIMEOUT_HOURS (default 2h): PID files now record
'{pid} {unix_timestamp}'; _mine_already_running() treats alive-but-old
processes as stale, unblocking queued mines after a ChromaDB hang.
Backward-compatible: bare-PID files (old format) treated as stale.
- Add MEMPALACE_MCP_IDLE_HOURS (default 8h): daemon watchdog thread in
mcp_server calls sys.exit(0) after the configured idle period, preventing
accumulation of stale server processes holding ChromaDB/HNSW file handles.
Set to 0 to disable.
- Enrich _internal_tool_error() with optional exc parameter: adds
data: {error_class, message} to JSON-RPC error body so callers can
distinguish lock contention, ChromaDB transients, and segfaults without
scraping the message string. MineAlreadyRunning handler in tool_sync()
adds error_class: 'LockHeldByOtherProcess' to the result dict.
- Update miner._cleanup_mine_pid_file() to parse first whitespace token
as PID (handles both old and new PID file formats).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review feedback: _MINE_PID_DIR is derived from STATE_DIR at module
import (hooks_cli.py:277), so patching STATE_DIR alone left
mine-spawning tests writing PID files under the import-time location
instead of the per-test root. Patch _MINE_PID_DIR too, and create the
state dir so the fixture's 'existing' docstring is accurate. Isolation
still 100 passed/1 skipped; ordering still 165 passed.
Nine save/log/precompact tests in test_hooks_cli.py passed only because
test_cli.py (alphabetically earlier) created ~/.mempalace in the session
tmp HOME as a side effect, satisfying the _palace_root_exists()
kill-switch. Run in isolation they short-circuited and failed (9 failed,
80 passed, 1 skipped).
Add a module autouse fixture that points PALACE_ROOT/STATE_DIR at a
per-test palace root that exists, so every test is robust standalone and
future tests don't inherit the trap. Kill-switch tests that need the
absent path call _redirect_palace_root after the fixture; monkeypatch
last-write-wins keeps their absent/file root and teardown restores the
real module value.
Isolation: pytest tests/test_hooks_cli.py -> 100 passed, 1 skipped.
Ordering preserved: test_cli + test_hooks_cli -> 165 passed.
Closes#1510
One-time mechanical reformat so `ruff format --check .` passes under the
newly pinned ruff. Layout only (assert-message parenthesization etc.),
no behavior change. 29 files: 28 under tests/ + 1 tools helper, no core
mempalace/ modules. Produced by `ruff format .`.
`_wing_from_transcript_path` derived the wing from the LAST dash-separated
token of Claude Code's encoded project folder. Because Claude Code encodes
the source directory by replacing `/` with `-`, any project whose folder
name itself contained a dash got silently truncated:
-Users-me-claude-code -> wing_code (lost "claude")
-Users-me-react-native -> wing_native (lost "react")
-Users-me-customer-portal -> wing_portal (collision risk)
-Users-me-admin-portal -> wing_portal (same wing!)
Two real consequences:
1. Project-scoped queries (`wake-up --wing <project>`) miss diary entries
because they're filed under the truncated wing.
2. Multi-project collision: any two projects whose folders end in the
same final token get their diary entries merged into one wing,
defeating the wing isolation model.
Fix uses a two-tier strategy:
1. PRIMARY — read `cwd` from the JSONL transcript. Claude Code records
the absolute working directory on most message types, so the project
name is whatever the leaf path segment of cwd is. This is the
canonical answer when present and never truncates hyphenated names.
Bounded scan (200 lines) keeps the lookup well within the hook's
500ms budget.
2. FALLBACK — when cwd isn't recorded (older Claude Code, queue-only
transcripts, etc.), decode the encoded folder. Strip the platform
user-home prefix (`Users-<user>-` / `home-<user>-`) and one common
parent-dir token (`git-`, `dev-`, `projects-`, `Projects-`, `src-`,
`code-`, `work-`, `Documents-`), then convert remaining dashes to
underscores. May include extra parent-dir noise in the wing name
(`wing_dev_mempalace_mempalace`) but never silently truncates.
Two existing tests asserted the old truncation behavior (it gave the
right answer by coincidence on single-token leaf project names). They're
updated to reflect the new contract: collision-safe wing extraction even
when cwd is absent.
Tests added:
- hyphenated_claude_code, hyphenated_react_native (regression)
- no_collision_between_hyphenated_siblings (`customer-portal` vs
`admin-portal` resolve to distinct wings)
- strips_parent_dir_with_hyphenated_project (reporter's example)
- uses_cwd_from_jsonl, cwd_with_hyphenated_project,
cwd_skips_lines_without_cwd, cwd_falls_back_when_no_cwd_in_jsonl,
cwd_handles_malformed_jsonl, cwd_handles_missing_file,
cwd_handles_non_string_cwd (cwd-primary path coverage)
Closes#1410.
The hook PID guard used a single global ``~/.mempalace/hook_state/mine.pid``
file, which failed two ways:
1. ``_mine_already_running`` read-then-spawn was a TOCTOU race. Two
near-simultaneous Stop hook fires both passed the existence/liveness
check before either wrote — so both ended up calling
``_spawn_mine``.
2. ``_spawn_mine`` unconditionally overwrote the global PID file with
the new child's PID. The first PID was lost, orphaning the first
child. The user-visible result in #1212 was two concurrent
``mempalace mine`` processes running against the same source, both
driving HNSW inserts in parallel — exactly the corruption pattern
the guard was meant to prevent. #1206 reported the same shape from
the perspective of the user (two mines hung on a 350MB folder).
Replace the global file with per-target slots under
``~/.mempalace/hook_state/mine_pids/``, keyed by sha256 of the mine
sub-arguments (everything after ``mine``). The slot is claimed via
``O_CREAT | O_EXCL`` so the claim is atomic — two simultaneous fires
can never both pass. Stale slots (PID exists but is dead) are
reclaimed transparently. Different targets (e.g. project mine vs
transcript ingest, or two different MEMPAL_DIRs) get independent
slots and run in parallel.
The mine subprocess receives its slot path via
``MEMPALACE_MINE_PID_FILE`` env var; ``miner._cleanup_mine_pid_file``
reads that var on exit and removes the slot if it points at our PID,
so orphaned slots from crashed mines don't accumulate.
Also routes ``_ingest_transcript`` through ``_spawn_mine`` so the
transcript ingest path now participates in the same dedup — repeated
Stop fires for the same transcript no longer stack parallel mines.
Closes#1212Closes#1206
The Stop hook spawns mining subprocesses via subprocess.Popen and then
returns. On Windows the parent stays blocked at session end because the
child inherits stdout/stderr handles and the OS waits for them to
release before the parent can exit — the user-visible symptom is the
"running stop hooks... 3/3" spinner hanging for minutes (#1268).
Add _detached_popen_kwargs() helper that returns the right detach knobs
per platform:
- POSIX: start_new_session=True, stdin=DEVNULL, close_fds=True
- Windows: creationflags=DETACHED_PROCESS|CREATE_NEW_PROCESS_GROUP|
CREATE_BREAKAWAY_FROM_JOB, stdin=DEVNULL, close_fds=True
Apply to all three fire-and-forget Popen sites in hooks_cli:
_spawn_mine, _ingest_transcript, _desktop_toast. Leave _mine_sync's
subprocess.run alone — that path is intentionally synchronous (the
precompact hook must wait for the mine to finish).
Note: the issue body references mempalace-stop.js, which does not exist
in this repo (the plugin ships shell wrappers calling Python). The
mechanism described — child holds parent open via inherited handles —
is universal, so this fix targets the equivalent symptom in our Python
hook path. Will follow up on the upstream JS file with the reporter.
The alias was placed below an explanatory comment block introduced by
#1305, which trips ruff E402 (module-level import not at top of file).
Moved next to the existing 'from mempalace.hooks_cli import (...)' line.
CI lint went red on develop after #1305 merged with the failing check;
this re-greens it so subsequent PRs do not inherit the failure.
Both @igorls and the Qodo bot flagged that `_palace_root_exists()` used
`Path.exists()`, which returns True for a regular file. A stray file at
`~/.mempalace` would let the kill-switch be bypassed and crash later in
`STATE_DIR.mkdir()` with NotADirectoryError.
Switched to `Path.is_dir()`. Also fold `_log()`'s inline check through
`_palace_root_exists()` so both kill-switch sites use the same predicate.
New test pins the behavior: a regular file at the palace root path is
treated as absent (hook short-circuits, _log does not crash, the stray
file is left untouched).
When the user removes ~/.mempalace/ (a strong "do not auto-capture"
signal), the next hook fire would silently recreate the entire dir
hierarchy and ingest existing transcripts:
1. _log() at hooks_cli.py:148 unconditionally calls
STATE_DIR.mkdir(parents=True, exist_ok=True), so the act of
writing the hook log line recreated ~/.mempalace/hook_state/
2. With no config file present, hook_stop_auto_save and
hook_precompact_auto_save defaulted to True (no override to read)
3. The full save path then ran, materializing palace/, wal/,
knowledge_graph.sqlite3, and N drawers from existing transcripts
in ~/.claude/projects/*.jsonl
All four entry points (hook_stop, hook_precompact, hook_session_start,
and _log itself) now check a new PALACE_ROOT = Path.home() / ".mempalace"
constant first and short-circuit (returning {} on stdout, never logging)
when the dir is absent. The user-removable directory is now a kill-switch.
Five unit tests in tests/test_hooks_cli.py cover: hook_stop /
hook_precompact / hook_session_start do not create the dir when absent;
_log() does not create it when absent; existing dir proceeds normally
(regression).
Caught in the wild on a downstream fork: ~146 drawers materialized in
under a second after a deliberate `rm -rf ~/.mempalace/`, into a planning
session that was explicitly not meant to be captured.
Address Copilot review on #1231:
1. Stop double-mining the transcript on the Python side. ``_get_mine_targets``
now returns only the ``MEMPAL_DIR`` projects target — the convos target
for the transcript dir is dropped because ``_ingest_transcript`` already
handles it on every hook fire. The duplicate spawn was using
``sys.executable`` (vs ``_mempalace_python()``) and a different ``--wing``,
so each Stop/PreCompact event was writing the same transcript into two
wings under asymmetric interpreters and overwriting the single
``_MINE_PID_FILE`` lock.
2. ``_maybe_auto_ingest`` and ``_mine_sync`` now spawn via
``_mempalace_python()`` so the resolved interpreter matches the venv
that owns mempalace (matters under GUI-launched harnesses where
``sys.executable`` may resolve to a system Python without chromadb).
3. Replace ``eval $(...)`` in both shell hooks with a ``mapfile``-based
reader. Sanitized values are still emitted by the same Python parser,
but the shell now does plain variable assignment instead of executing
the parser's stdout — smaller blast radius if the sanitizer is ever
bypassed.
4. Mirror ``_validate_transcript_path`` in the shell hooks via a
``is_valid_transcript_path`` helper — extension + traversal-segment
rejection, parity with the Python validator. The convos mine in each
shell hook is now gated on the validator instead of bare ``-f``.
5. Tighten the ``..`` traversal test that previously exercised the
suffix gate by mistake (``../../etc/passwd`` lacks ``.json[l]``).
Use ``.jsonl`` paths with traversal segments to actually hit the
``..`` rejection branch.
6. README: add a one-liner pointing at ``mempalace sweep`` for users
who want per-message recall on top of the file-level chunks the
hooks produce. The sweeper was undiscoverable previously.
Tests: 1418 passed, 1 skipped (full suite minus benchmarks).
#1230 fixed --mode convos for the case where MEMPAL_DIR was unset, but
left two configurations broken:
- MEMPAL_DIR set to a project dir: convos never mined (MEMPAL_DIR
overrode the transcript path); only project files were ingested.
- MEMPAL_DIR set to a conversations dir per the old hooks/README: the
projects miner ran on JSONL — same wrong-miner behaviour.
The shell hooks (mempal_save_hook.sh, mempal_precompact_hook.sh) had
the same MEMPAL_DIR-overrides-transcript bug AND were missing --mode
on every spawned `mempalace mine` call.
Make the auto-ingest *additive*. _get_mine_dir → _get_mine_targets,
returning a list of (dir, mode) pairs:
- MEMPAL_DIR (when valid) contributes (dir, "projects")
- A valid transcript JSONL contributes (parent, "convos")
- Both can appear together; the hook spawns one ingest per target
Same change applied to the shell save and precompact hooks. Precompact
also gained transcript_path parsing so it can run the convos mine
synchronously before context is compressed. hooks/README.md updated to
describe MEMPAL_DIR as a project-files target, never a convos target.
- Normalize MEMPAL_DIR via Path.expanduser().resolve() so ~/proj paths
are correctly accepted instead of falling through to transcript fallback
- Replace bare Path.expanduser().is_file() transcript check with the
existing _validate_transcript_path() which adds .resolve(), enforces
.jsonl/.json extension, and rejects '..' path-traversal components
- Update tests to compare resolved paths (cross-platform correctness)
- Add tests for tilde expansion, path-traversal rejection, and
non-jsonl extension rejection in _get_mine_dir
Agent-Logs-Url: https://github.com/MemPalace/mempalace/sessions/f69176c7-d752-40ef-ba71-d0e4adc3a689
Co-authored-by: igorls <4753812+igorls@users.noreply.github.com>
The Stop and PreCompact hooks spawn `mempalace mine <dir>` with no
`--mode` flag, which defaults to `projects` in cli.py. When MEMPAL_DIR
is unset, _get_mine_dir falls back to the parent of the transcript
JSONL — and miner.py's READABLE_EXTENSIONS includes `.jsonl`, so the
projects miner happily ingests Claude Code session JSONL as if it were
source code instead of conversation.
Make _get_mine_dir return (dir, mode): MEMPAL_DIR keeps `projects`,
the JSONL fallback yields `convos`. Both _maybe_auto_ingest and
_mine_sync now thread the mode into the spawned command.
_wing_from_transcript_path only matched '-Projects-<name>' segments,
so Linux users with code under ~/dev/, ~/code/, or ~/src/ fell through
to the wing_sessions fallback and lost the per-project diary scoping
introduced in #659.
Broaden the heuristic to derive the project from the final
dash-separated token of the encoded project-folder name under
.claude/projects/. Keeps the legacy -Projects- regex as a secondary
match for transcripts living outside the standard Claude Code path.
Covers macOS Users layout, Linux dev/code layouts, and deeper nested
source paths while preserving existing Projects/ behavior.
* fix: add wing param to diary_write/diary_read, derive from transcript path
Without a wing override, all diary entries from the stop hook land in
wing_session-hook regardless of which project the session is in, making
per-project diary search impossible.
- tool_diary_write(): add optional `wing` param; sanitize and use it when
provided, fall back to wing_{agent_name} when omitted
- tool_diary_read(): add optional `wing` param for filtering by target wing
- TOOLS dict: expose `wing` in input_schema for both diary tools
- hooks_cli: add _wing_from_transcript_path() helper that extracts the
project name from Claude Code paths like
~/.claude/projects/-home-jp-Projects-kiyo-xhci-fix/... → kiyo-xhci-fix
- hook_stop: derive project wing and append wing= hint to block reason so
Claude writes diary entries to the correct per-project wing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: sanitize wing param, cross-platform paths, tighten test assertions
Addresses Copilot review feedback on #659.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: wing_ prefix + agent filter on diary_read
Addresses bensig's 2-issue review on this PR.
1. _wing_from_transcript_path() was returning bare project names
(e.g. "myproject") while all existing wings follow the wing_*
convention from AAAK_SPEC. Entries landed in wing="myproject"
while diary_read defaulted to wing="wing_<agent_name>" —
orphaning every diary entry written by the stop hook. Now
returns "wing_<project>" and falls back to "wing_sessions".
2. tool_diary_read() did not include agent_name in the ChromaDB
where filter when a custom wing was provided — any caller with
a shared wing could read entries written by other agents.
Add {"agent": agent_name} to the $and clause. Also flagged by
Qudo and left unresolved until now.
Tests updated to expect the wing_ prefix (6 tests).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Clean squash by jphein on 2026-04-21. Backwards-compatible via hook_silent_save config flag. Save marker now only advances after confirmed write — strictly safer than status quo.
Adds a `hook_silent_save` mode (default `true` in new installs) where
the stop and precompact hooks write diary entries directly via the
Python API — no AI block, no MCP tool roundtrip, no possibility of the
AI forgetting or ignoring the save instruction.
**Two modes, controlled by `hook_silent_save` in `~/.mempalace/config.json`:**
1. **Silent mode** (default): Direct call to `tool_diary_write()`. Plain
text, no AI involved, deterministic. Save marker advances only after
the write is confirmed, so mid-save failures do not lose exchanges.
Shows `"✦ N memories woven into the palace"` as a systemMessage
notification so the user knows the save fired.
2. **Block mode** (legacy): Returns `{"decision": "block"}` asking the
AI to call the MCP tool chain. Non-deterministic — the AI may ignore,
summarize lossy, or fail. Kept for backward compatibility.
**Extras rolled in:**
- Block reasons name "MemPalace" explicitly and instruct the AI not to
write to Claude Code's native auto-memory (.md files) — prevents the
two memory systems from stepping on each other.
- Codex transcript handling (`event_msg` payloads) in
`_count_human_messages` + `_extract_recent_messages`.
- Tightened stopword leak in diary summaries; docstring polish; test
hermeticity fixes (per-test `STATE_DIR` patching).
**Tests:** hooks_cli tests cover silent-save path, save-marker
advancement after confirmed write only, and systemMessage formatting.
Rebased fresh on upstream/develop. Only touches files germane to the
feature (hooks_cli.py, tests, hooks/README.md, HOOKS_TUTORIAL.md) —
stale fork-local `.sh` wrapper and plugin manifest changes dropped.
Every stop hook fire spawned a new background `mempalace mine` via
subprocess.Popen with no dedup — 4 concurrent mines at ~770% CPU
observed in production. Add `_mine_already_running()` (reads
`hook_state/mine.pid`, uses `os.kill(pid, 0)` as an existence check)
and `_spawn_mine()` (writes the child PID to the lock file after
Popen returns). `_maybe_auto_ingest` bails early when the guard
reports True.
Tests: 4 new unit tests for `_mine_already_running` (no file, dead
PID, live PID using `os.getpid()`, corrupt file), 1 new test
covering the skip-when-running branch of `_maybe_auto_ingest`, and
existing spawn tests patched to redirect `_MINE_PID_FILE` into
tmp_path so they don't touch the real state dir.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- _output(): use sys.modules.get() instead of unconditional import to
avoid triggering mcp_server's stdout redirect as a side effect
- _output(): write-all loop for os.write() to handle partial writes and
EINTR; fall back to sys.stdout.buffer on OSError
- _output() docstring: remove inaccurate _save_diary_direct reference
- stop_hook_active guard: narrow except to ImportError/AttributeError,
default silent_guard=False (safe: preserves block-mode loop prevention
when config load fails) and log a warning instead of silently changing
behavior
- tests: two new regression tests covering the real-stdout-fd path and
the fd-1 fallback path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(hooks): stop precompact hook from blocking compaction
The precompact hook unconditionally returned {"decision": "block"},
which in Claude Code means "cancel compaction" with no retry mechanism.
This made /compact permanently broken for all plugin users.
Changed hook_precompact() to mine the transcript synchronously (so data
lands before compaction) and return {"decision": "allow"}. This matches
the standalone bash hook in hooks/ which already uses allow.
Also extracted _get_mine_dir() and _mine_sync() helpers so precompact
can mine from the transcript directory, not just MEMPAL_DIR.
Stop hook behavior is unchanged -- left for #673 which implements the
full silent save path.
Closes#856, closes#858.
* fix: use empty JSON instead of invalid \"allow\" decision value
Claude Code only recognizes \"block\" as a top-level decision value.
\"allow\" is a permissionDecision value for PreToolUse hooks, not a
valid top-level decision. The correct way to not block is to return
empty JSON. Caught by #872.
- _count_human_messages() now logs a WARNING via _log() when a
non-empty transcript_path is rejected by the validator, making
silent auto-save failures diagnosable via hook.log
- Add test for platform-native paths (backslashes on Windows) to
verify _validate_transcript_path works cross-platform
- Add test verifying the warning log is emitted on rejection
Refs: MemPalace/mempalace#809
Sandro da Silva
mvalentsev
Ivan Antsimonau
Igor Lins e Silva
eldar702
sha2fiddy
Milla J
Arnold Wender
lcatlett
copilot-swe-agent[bot]
Jeffrey Hein
Ben Sigman
BLUDATA\marcio.heiderscheidt
Tal Muskal
Tal Muskal