fix(3.3.3): two followups from #1145 before tag cut#1147
Conversation
_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.
#1097 fixed mempalace_search to treat empty-string wing/room as no filter, matching how LLM agents default to filling every optional parameter with ''. The same pattern wasn't applied to diary_read: passing wing='' defaulted to wing_<agent_name>, siloing away entries that hooks had written to project-derived wings per #659. When wing is empty/omitted, filter only on agent + room=diary so callers get a unified view of the agent's journal across every wing it has written to. Explicit wing=<name> continues to scope reads to that wing only. Adds test covering empty-wing read after writing to both the default and a non-default wing.
Two follow-up fixes from the v3.3.3 smoke test get folded into 3.3.3 before the tag is cut. Also syncs uv.lock with the 3.3.3 version bump merged via #1144.
There was a problem hiding this comment.
Pull request overview
This PR adds two follow-up fixes intended to ship in the v3.3.3 release: broadening Claude Code transcript → wing derivation to support non-~/Projects layouts, and aligning mempalace_diary_read behavior so wing="" means “no wing filter” (return across wings) rather than defaulting to wing_<agent>.
Changes:
- Update
_wing_from_transcript_pathto derive the project wing from the encoded.claude/projects/folder name, plus add tests for additional layouts. - Change
tool_diary_readsowing=""returns diary entries across all wings for the given agent, and add a regression test. - Update changelog and lockfile for the 3.3.3 release.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
mempalace/hooks_cli.py |
Updates project-wing derivation logic from transcript paths. |
mempalace/mcp_server.py |
Adjusts diary read filtering semantics when wing is empty. |
tests/test_hooks_cli.py |
Adds tests covering additional transcript-path layouts. |
tests/test_mcp_server.py |
Adds coverage for cross-wing diary reads when wing="". |
CHANGELOG.md |
Documents the two follow-up bug fixes under 3.3.3. |
uv.lock |
Bumps mempalace version to 3.3.3 in the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Primary: pull the encoded project folder out of ``.claude/projects/`` | ||
| # and take its last dash-separated token. | ||
| match = re.search(r"/\.claude/projects/-([^/]+)", normalized) | ||
| if match: | ||
| encoded = match.group(1) | ||
| project = encoded.rsplit("-", 1)[-1] | ||
| if project: | ||
| return f"wing_{project.lower().replace(' ', '_')}" | ||
| # Legacy fallback: explicit ``-Projects-<name>`` segment, useful for | ||
| # transcripts not under the standard Claude Code projects dir. | ||
| match = re.search(r"-Projects-([^/]+?)(?:/|$)", normalized) |
There was a problem hiding this comment.
_wing_from_transcript_path’s new “last dash-separated token” heuristic breaks existing Claude Code paths that include -Projects- when the project directory name itself contains dashes (e.g. .../-home-jp-Projects-kiyo-xhci-fix/... will return wing_fix). Since the legacy -Projects-<name> extraction preserves hyphenated project names, prefer it when present (or at least run that match before the generic .claude/projects/-... match), and add a regression test for a hyphenated -Projects- project name.
| @@ -995,10 +995,11 @@ 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. | |||
There was a problem hiding this comment.
Docstring says diary_read returns the last N entries “in chronological order”, but the implementation sorts with reverse=True and returns newest-first. Either update the docstring to say reverse-chronological (most recent first) or change the sort/slicing to return chronological order as documented.
| in chronological order — the agent's personal journal. | |
| in reverse-chronological order (most recent first) — the agent's | |
| personal journal. |
| @@ -1007,21 +1008,20 @@ def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""): | |||
| except ValueError as e: | |||
There was a problem hiding this comment.
tool_diary_read’s wing normalization only treats the empty string as “no filter”; a whitespace-only wing (e.g. " ") is truthy and will currently raise from sanitize_name, which is inconsistent with the repo’s established optional-filter helper _sanitize_optional_name (it treats None or any not value.strip() as no filter). Consider using _sanitize_optional_name(wing, "wing") here to match the #1097 convention more closely.
…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) <noreply@anthropic.com>
bensig
left a comment
bensig
left a comment
There was a problem hiding this comment.
Read + ran locally against `fix/3.3.3-followups` (commits d158375, 1fd16da, 6fcfd34). LGTM, ship it.
Diff review
Bug 1 — `_wing_from_transcript_path` broadening (hooks_cli.py)
- Primary path: match `/.claude/projects/-` and take the final dash-separated token. Covers macOS `
/Projects/`, Linux `/dev/`, `/code/`, `/src/`, and nested layouts. Correct shape. - Legacy `-Projects-` regex retained as secondary fallback for transcripts outside the standard Claude Code projects dir. Defensive, doesn't interfere with the primary.
- Three new unit tests cover non-Projects Linux, macOS users-folder, and nested-deep. Good coverage.
Bug 2 — `tool_diary_read(wing='')` empty-string normalization (mcp_server.py)
- Removes the `wing_` auto-fallback; builds a `conditions` list that always scopes by `room=diary` + `agent` and only adds `wing` when truthy.
- Matches the #1097 pattern applied to `mempalace_search`. Semantically consistent.
- Subtle behavior change worth noting in docstring (which you did): old default was "this agent's default wing"; new default is "all wings this agent has written to." The new default is clearly right given hooks write to project-derived wings via #659 — entries would otherwise be invisible.
- Test covers both the empty-wing broadening case and the explicit-wing still-scoped case. Good.
Local verification
```bash
$ pytest tests/ --ignore=tests/benchmarks --cov=mempalace --cov-fail-under=80 -q
1075 passed, 1 warning in 76.62s
Required test coverage of 80% reached. Total coverage: 83.35%
```
All green. 4 new tests land cleanly (3 for bug 1, 1 for bug 2).
CHANGELOG
Already updated in 6fcfd34 with accurate bullets in the `[3.3.3]` section. Nothing to add.
Ready for tag as soon as this merges. Thanks for the thorough smoke test.
…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) <noreply@anthropic.com>
… state Three stale sections updated: - Fork change queue: row 8 (.blob_seq_ids_migrated marker) struck through → FILED as MemPalace#1177. Two new rows added for segfault fixes discovered today (MemPalace#1171 concurrent-write lock, MemPalace#1173 quarantine in make_client) that weren't in the queue because the bugs surfaced today, not during the original 2026-04-21 triage. - Open upstream PRs: was showing 3 of 10 PRs. Now shows all 10 with current CI/review state. All rebased onto current upstream/develop and MERGEABLE as of today. - Merged since v3.3.1: added v3.3.3 release (2026-04-24) with its constituent merges — MemPalace#942, MemPalace#833, MemPalace#1097, MemPalace#1145, MemPalace#1147, MemPalace#1148/1150/1157 entity-detection overhaul (via @igorls's MemPalace#1175 stacked-PR rescue), MemPalace#1166 palace-path security, MemPalace#340/MemPalace#1093 install regression, plus MemPalace#851 from the 2026-04-22 batch.
v3.3.3 — restore install integrity See CHANGELOG.md for the full release notes. Headline fix: the mempalace-mcp console-script drift in v3.3.2. The plugin.json referenced a binary that pyproject.toml never declared, so every fresh 'pip install mempalace==3.3.2' produced a broken Claude Code plugin config. Fixed by MemPalace#340 (messelink) and follow-ups in MemPalace#1147 (igorls). # Conflicts: # CHANGELOG.md # mempalace/hooks_cli.py # tests/test_hooks_cli.py
Two follow-up fixes surfaced by the v3.3.3 smoke test, landing before the
v3.3.3tag is cut so they ship in this release rather than 3.3.4.Filed as #1145. The env-propagation observation in that issue was a measurement artifact from concurrent background mines on different palaces — verified
MEMPALACE_PALACE_PATHis honored bysubprocess.Popen(inherits parent env by default). No code change needed for that one.Summary
1.
_wing_from_transcript_path— broaden beyond macOS~/Projects/mempalace/hooks_cli.py:490only matched-Projects-<name>in encoded transcript folders, so Linux users with code under~/dev/,~/code/,~/src/fell through to thewing_sessionsfallback and lost the per-project diary scoping that #659 was meant to deliver.Fix derives the project name from the final dash-separated token of the encoded folder under
.claude/projects/— this covers macOS/Users/<user>/…/<project>, Linux/home/<user>/dev/<parent>/<project>, and nested layouts while preserving the legacy-Projects-match as a secondary fallback for transcripts outside the standard Claude Code path.2.
mempalace_diary_read(wing="")— return entries across all wingsAfter #659, hook-driven diary writes land in
wing_<project>instead of the agent's defaultwing_<agent>. Butdiary_readwithwing=""defaulted towing_<agent>, invisible-ing those hook-written entries.Fix applies the #1097 "empty-string as no filter" pattern: when
wingis empty, filter byagent + room=diaryonly. Explicitwing=<name>continues to scope to that wing.Verification
uv run pytest tests/— 1075 passed (includes 4 new tests: 3 wing-derivation layouts, 1 cross-wing diary read)uvx --from 'ruff>=0.4.0,<0.5' ruff check— All checks passed (CI parity)uvx --from 'ruff>=0.4.0,<0.5' ruff format --check— 4 files already formattedChangelog
Entries appended to the existing
[3.3.3]Bug Fixes section — no new version section, since these ship as part of 3.3.3.Sequencing
The
v3.3.3tag hasn't been pushed yet (confirmed againstgit ls-remote --tags origin). Merging this beforegit tag -a v3.3.3folds both fixes into the release.