fix: diary wing routing for non-Projects layouts + empty-wing reads (#1145 bugs 1 & 2) by jphein · Pull Request #1146 · MemPalace/mempalace

jphein · 2026-04-24T02:36:34Z

Addresses two of the three findings @igorls flagged on #1145 from the v3.3.3 smoke test. Batched into one PR because both are in the same file class and share the "LLM agent / non-~/Projects/ user defaults to empty string" failure mode. Bug 3 (Stop-hook auto-mine MEMPALACE_PALACE_PATH env propagation) lives in a different code path and is deferred to a follow-up PR pending local repro.

Bug 1 — `_wing_from_transcript_path` is `~/Projects/`-centric

The heuristic only matched -Projects-<project> in the encoded transcript folder name. Users whose source lives outside ~/Projects/ — ~/dev/, ~/src/, ~/code/, or any other Linux convention — got transcript paths like ~/.claude/projects/-home-<user>-dev-<Parent>-<project>/*.jsonl with no -Projects- segment, fell through to the wing_sessions fallback, and lost the per-project diary scoping #659 was meant to deliver.

Fix: keep the explicit -Projects-<x> match first (preserves existing behavior / test coverage), then fall back to the last --delimited segment of the encoded folder name. That segment is the actual project directory name regardless of source layout — Claude Code's encoding puts it there for any path, not just under ~/Projects/.

Bug 2 — `tool_diary_read` didn't adopt #1097's empty-string pattern

#1097 fixed mempalace_search to treat wing="" / room="" as "no filter" (LLM agents frequently default optional string parameters to ""). tool_diary_read still defaulted an empty wing to wing_<agent_name>, silently hiding entries written to named wings — e.g., the per-project wings from #659 that the stop-hook now derives.

Fix: route wing through the _sanitize_optional_name() helper #1097 introduced so empty / whitespace / None all coerce to None and drop out of the query. An explicit non-empty wing still scopes to that wing. Build the where clause from a list of required conditions (room=diary, agent=<name>) and conditionally append the wing filter — matches the pattern tool_list_drawers already uses.

Tests

Six new tests, all green. Full suite: 1067 passed (this branch, from upstream/develop).

tests/test_hooks_cli.py — four _wing_from_transcript_path cases:

test_wing_from_transcript_path_non_projects_layout — igor's specific scenario (~/dev/<Parent>/<project>)
test_wing_from_transcript_path_src_layout — ~/src/<app>
test_wing_from_transcript_path_code_layout — ~/code/<app>
test_wing_from_transcript_path_bare_folder_no_delimiters_falls_back — genuinely unstructured parent → wing_sessions

tests/test_mcp_server.py::TestDiaryTools — two tool_diary_read cases:

test_diary_read_empty_wing_returns_across_wings — wing="" returns entries from any wing (the bug as reported)
test_diary_read_explicit_wing_still_filters — wing="wing_a" still scopes correctly; regression guard against overcorrecting the empty-string fix

Not in this PR

Bug 3 from #1145 (Stop-hook auto-mine subprocess ignores MEMPALACE_PALACE_PATH env) — env-propagation bug in a different subprocess-launch site, same file but different narrative/tests. Deferred to a follow-up PR once I've reproduced locally and confirmed root cause. Happy to bundle if reviewers prefer one PR; say the word.

🤖 Generated with Claude Code

@igorls

Addresses two of the three findings @igorls flagged in MemPalace#1145 from the v3.3.3 smoke test. Same file class, same "LLM agent / edge-case user defaults to empty string" failure mode — batched into one PR. ## Bug 1: `_wing_from_transcript_path` regex was too narrow The regex only matched `-Projects-<project>` in the encoded transcript folder name. Users whose code lives outside `~/Projects/` — `~/dev/`, `~/src/`, `~/code/`, or any non-Projects convention — got transcript paths like `~/.claude/projects/-home-<user>-dev-<Parent>-<project>/...` with no `-Projects-` segment, fell through to `wing_sessions`, and lost the per-project diary scoping MemPalace#659 was meant to deliver. Fix: add a second strategy. Keep the explicit `-Projects-<x>` match first (preserves existing test coverage and behavior for code under `~/Projects/`), then fall back to the last `-`-delimited segment of the encoded folder name. That segment is the actual project directory name regardless of source layout. Four new tests cover the non-Projects scenarios igor raised plus a genuine fallback case (parent folder without `-` delimiters → still `wing_sessions`). Existing tests for the `-Projects-<x>` path stay green. ## Bug 2: `tool_diary_read` ignored MemPalace#1097's empty-string pattern `MemPalace#1097` fixed `mempalace_search` to treat `wing=""` / `room=""` as "no filter" — LLM agents frequently default optional string parameters to `""`. `tool_diary_read` still defaulted an empty `wing` to `wing_<agent_name>`, which silently hid entries written to named wings (e.g. the per-project wings from MemPalace#659). Fix: route `wing` through `_sanitize_optional_name()` (the helper MemPalace#1097 introduced) so empty / whitespace / `None` all coerce to `None` and drop out of the query filter. An explicit non-empty wing still scopes to that wing — verified by a regression test. Build the `where` clause from a list of required conditions (`room=diary`, `agent=<name>`) and conditionally append the wing filter — matches the pattern `tool_list_drawers` already uses. ## Bug 3 (Stop-hook auto-mine `MEMPALACE_PALACE_PATH` env propagation) — not in this PR Deferred to a follow-up PR pending local reproduction of the env- propagation behavior. Will file separately once I've confirmed root cause per the plan in my MemPalace#1145 comment. ## Tests 1067 passed on this branch (branched from `upstream/develop`). Six new tests: - tests/test_hooks_cli.py — four new `_wing_from_transcript_path` cases (`~/dev/`, `~/src/`, `~/code/`, bare fallback) - tests/test_mcp_server.py — two new `tool_diary_read` cases (empty-wing reads across wings, explicit wing still filters) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Copilot

Pull request overview

Fixes two hook/diary routing issues triggered by non-~/Projects/ source layouts and LLM agents defaulting optional parameters to empty strings, improving per-project diary scoping and cross-wing diary reads.

Changes:

Extend _wing_from_transcript_path() to derive a project wing for non-~/Projects/ transcript folder encodings.
Update tool_diary_read() to normalize wing using _sanitize_optional_name() and conditionally apply the wing filter.
Add unit tests covering non-Projects transcript layouts and empty-wing diary reads.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`mempalace/hooks_cli.py`	Adds a second heuristic for deriving project wings from encoded transcript folder names outside `~/Projects/`.
`mempalace/mcp_server.py`	Changes diary read filtering to treat empty/whitespace/None wing as “no filter” and builds the `where` clause conditionally.
`tests/test_hooks_cli.py`	Adds coverage for wing derivation across common non-Projects directory layouts plus a fallback case.
`tests/test_mcp_server.py`	Adds coverage ensuring `wing=""` reads across wings while explicit wing still scopes correctly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-24T02:40:39Z

 def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""):
    """
    Read an agent's recent diary entries. Returns the last N entries
    in chronological order — the agent's personal journal.

-    When ``wing`` is provided, reads from that wing instead of the
-    agent's default ``wing_<agent_name>`` wing.  This lets hooks
-    direct diary reads to a project-specific wing derived from
-    the transcript path.
+    ``wing`` behavior matches ``mempalace_search`` (#1097):
+    empty-string / whitespace-only / ``None`` means "no wing filter" —
+    return entries from any wing this agent has written to. An explicit
+    non-empty wing scopes reads to that wing only, e.g. for hooks that
+    direct diary reads to a project-specific wing derived from the
+    transcript path. Resolves #1145 bug 2.
    """
    try:
        agent_name = sanitize_name(agent_name, "agent_name")
-        if wing:
-            wing = sanitize_name(wing)
+        wing = _sanitize_optional_name(wing, "wing")
    except ValueError as e:


tool_diary_read now treats the default wing="" (i.e., callers omitting wing) as “no wing filter”, but the tool schema still documents “If omitted, reads from wing_{agent_name}”, and this is a behavioral change from the previous default-to-agent-wing behavior. If you want to support both behaviors (omit => agent default, explicit empty string/whitespace => no filter), consider using a sentinel default (e.g., wing=_UNSET) to distinguish “not provided” from "", and update the TOOLS["mempalace_diary_read"] schema description to match the actual semantics.

Copilot · 2026-04-24T02:40:40Z

    try:
+        # Build filter conditions: agent + diary room always apply; wing is
+        # optional (matches #1097's empty-string semantics for tool_search).
+        conditions = [
+            {"room": "diary"},
+            {"agent": agent_name},
+        ]
+        if wing:
+            conditions.append({"wing": wing})
+        where = conditions[0] if len(conditions) == 1 else {"$and": conditions}
        results = col.get(
-            where={
-                "$and": [
-                    {"wing": wing},
-                    {"room": "diary"},
-                    {"agent": agent_name},
-                ]
-            },
+            where=where,
            include=["documents", "metadatas"],
            limit=10000,
        )


With wing optional (and potentially unset/empty), tool_diary_read can now fetch across all wings for an agent, but it still does col.get(..., limit=10000) and then sorts/slices client-side. This risks silently truncating results (there’s an existing note elsewhere about a 10K get() limit) and makes the total field inaccurate when there are >10K diary entries. Consider paginating get() (offset loop) or fetching only metadatas first to identify the latest last_n entry IDs, then retrieving documents for just those IDs; this avoids large reads and removes the 10K correctness hazard.

jphein · 2026-04-24T02:43:49Z

Closing as duplicate — we filed #1146 and @igorls filed #1147 within four minutes of each other covering the same two bugs. Their PR is the right vehicle for 3.3.3:

Cleaner primary regex — /\.claude/projects/-([^/]+) matches Claude Code's actual encoded-folder location as the primary strategy, with -Projects-<x> as the legacy fallback. That reflects the encoding rules more directly than my "-Projects- first, last-segment fallback" ordering.
CHANGELOG updated — the 3.3.3 followup context is preserved in the release notes where it belongs.
Docstring update on tool_diary_read is clearer about the "empty = no filter" contract.
Tests cover the same ground; theirs have the macOS /Users/ layout case which mine didn't.

Thanks @igorls for the smoke test + quick turnaround on both the findings and the fix. Re the Bug 3 retraction — confirmed on my side: subprocess.Popen without env= inherits parent env, MempalaceConfig.palace_path reads MEMPALACE_PALACE_PATH / MEMPAL_PALACE_PATH before falling back to config-file default. No propagation gap. Good catch on the measurement artifact.

No action needed on my end.

@igorls

…alace#1147 Filed MemPalace#1146 at 02:36 UTC, @igorls filed MemPalace#1147 at 02:40 UTC covering the same two bugs from MemPalace#1145. Their approach is cleaner (primary regex pattern-matches Claude Code's `.claude/projects/-` location directly; ours used `-Projects-<x>` first with fallback). Bug 3 retracted — confirmed env inheritance works via `subprocess.Popen` default, no propagation gap. Fork main keeps `34e36ae` for local use until upstream merges MemPalace#1147 as part of v3.3.3; next `develop→main` merge will replace our version with upstream's. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

@igorls

…g + empty-wing reads) 4 commits pulled (8ac98f0, 6fcfd34, 1fd16da, d158375) landing @igorls's MemPalace#1147 which fixes the same two MemPalace#1145 bugs we addressed in our 34e36ae (closed as duplicate by MemPalace#1146 close). Conflict resolution — took upstream on all 4 overlapping files: - mempalace/hooks_cli.py — upstream's `.claude/projects/-` primary regex with `-Projects-<x>` legacy fallback (functionally equivalent to ours, slightly cleaner ordering) - mempalace/mcp_server.py — upstream's tool_diary_read empty-wing normalization (essentially identical to ours; docstring is clearer) - tests/test_hooks_cli.py — upstream's tests (3 new cases: non-Projects layout, macOS Users/, nested deep). Lost our bare-folder-no-delimiters test, but upstream's coverage is adequate. - tests/test_mcp_server.py — upstream's diary_read tests (cover wing="" spans all wings + explicit wing filter). Overlap with ours. Post-merge: 1094 passed + 10 expected fork-ahead MemPalace#19 failures in test_claude_plugin_hook_wrappers.py (venv-aware hooks vs PATH-only test contract — documented, unchanged). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Copilot AI review requested due to automatic review settings April 24, 2026 02:36

jphein requested review from bensig, igorls and milla-jovovich as code owners April 24, 2026 02:36

Copilot started reviewing on behalf of jphein April 24, 2026 02:37 View session

jphein added a commit to jphein/mempalace that referenced this pull request Apr 24, 2026

docs(CLAUDE.md): track MemPalace#1146 — MemPalace#1145 bugs 1+2 PR open

6f5da49

Copilot AI reviewed Apr 24, 2026

View reviewed changes

jphein closed this Apr 24, 2026

jphein deleted the fix/wing-derivation-and-diary-read-empty-wing branch April 25, 2026 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: diary wing routing for non-Projects layouts + empty-wing reads (#1145 bugs 1 & 2)#1146

fix: diary wing routing for non-Projects layouts + empty-wing reads (#1145 bugs 1 & 2)#1146
jphein wants to merge 1 commit intoMemPalace:developfrom
jphein:fix/wing-derivation-and-diary-read-empty-wing

jphein commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

jphein commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jphein commented Apr 24, 2026

Bug 1 — _wing_from_transcript_path is ~/Projects/-centric

Bug 2 — tool_diary_read didn't adopt #1097's empty-string pattern

Tests

Not in this PR

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

jphein commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bug 1 — `_wing_from_transcript_path` is `~/Projects/`-centric

Bug 2 — `tool_diary_read` didn't adopt #1097's empty-string pattern