fix: add wing param to diary_write/diary_read, derive from transcript path by jphein · Pull Request #659 · MemPalace/mempalace

jphein · 2026-04-12T02:42:35Z

Problem

All diary entries from the stop hook land in wing_session-hook regardless of which project the Claude Code session is in. This makes per-project diary search impossible — everything piles into one wing.

Solution

`mcp_server.py`

Add optional wing parameter to tool_diary_write(). If provided, sanitizes it (lowercase, spaces→underscores) and uses it directly. If omitted, falls back to the existing wing_{agent_name} behavior.
Add optional wing parameter to tool_diary_read() for filtering by target wing.
Expose wing in the input_schema for both mempalace_diary_write and mempalace_diary_read in the TOOLS dict.

`hooks_cli.py`

Add _wing_from_transcript_path(path) helper that extracts the project name from Claude Code transcript paths:
- ~/.claude/projects/-home-jp-Projects-kiyo-xhci-fix/... → kiyo-xhci-fix
- Falls back to "sessions" for unrecognized paths.
In hook_stop, derive the project wing from the transcript path and append a wing=<project> hint to the block reason so Claude writes diary entries to the correct per-project wing.

Backwards Compatibility

Fully backwards compatible — wing is optional in both functions and the TOOLS schema. Omitting it produces identical behavior to before.

Test Changes

Updated test_stop_hook_blocks_at_interval to use startswith(STOP_BLOCK_REASON) since the block reason now includes a project wing suffix.

Test plan

python -m pytest tests/ -q --ignore=tests/test_convo_miner.py — all 597 tests pass
ruff check mempalace/mcp_server.py mempalace/hooks_cli.py — no issues
Manual: tool_diary_write(agent_name="claude", entry="test", wing="my_project") stores in my_project/diary
Manual: tool_diary_write(agent_name="claude", entry="test") still stores in wing_claude/diary (backwards compat)
Manual: stop hook block reason includes wing=<project> for Claude Code sessions

Copilot

Pull request overview

This PR fixes stop-hook diary entries being written to a single agent wing by introducing an optional wing parameter for diary read/write and deriving a per-project wing from the Claude Code transcript path.

Changes:

Added optional wing parameter to tool_diary_write() and tool_diary_read(), and exposed it via the MCP tool schemas.
Added transcript-path parsing in the stop hook to append a wing=<project> hint to the block reason.
Relaxed the stop-hook test assertion to account for the new reason suffix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`mempalace/mcp_server.py`	Adds `wing` support to diary tools and updates MCP tool schemas accordingly.
`mempalace/hooks_cli.py`	Derives a per-project wing from transcript paths and includes it in stop-hook block messaging.
`tests/test_hooks_cli.py`	Updates assertion to tolerate the new stop-hook reason suffix.

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

Copilot · 2026-04-12T02:45:33Z

    try:
        agent_name = sanitize_name(agent_name, "agent_name")
        entry = sanitize_content(entry)
    except ValueError as e:
        return {"success": False, "error": str(e)}

-    wing = f"wing_{agent_name.lower().replace(' ', '_')}"
+    if wing:
+        wing = wing.lower().replace(" ", "_")
+    else:
+        wing = f"wing_{agent_name.lower().replace(' ', '_')}"
    room = "diary"


wing is currently only normalized via lower().replace(" ", "_") and is not passed through sanitize_name like other wing/room inputs (e.g. tool_add_drawer). This allows slashes, .., very long strings, etc., which can break IDs/metadata and bypass the repo’s path-traversal protections. Please validate/sanitize wing (and ideally the final computed wing string) using sanitize_name, and treat whitespace-only wing as “not provided”.

Fixed — replaced wing.lower().replace(" ", "_") with sanitize_name(wing) for proper path-traversal/length/character validation.

Copilot · 2026-04-12T02:45:33Z

+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.
    """
-    wing = f"wing_{agent_name.lower().replace(' ', '_')}"
+    if wing:
+        wing = wing.lower().replace(" ", "_")
+    else:
+        wing = f"wing_{agent_name.lower().replace(' ', '_')}"
    col = _get_collection()


tool_diary_read now accepts a user-provided wing but does not validate it (or agent_name) and doesn’t handle ValueError like tool_diary_write. For consistency and to avoid invalid metadata queries / unexpected behavior, sanitize agent_name and wing with the shared sanitizers and return a structured {success: False, error: ...} on validation failure.

Fixed — added sanitize_name() for both agent_name and wing in tool_diary_read(), plus try/except ValueError matching tool_diary_write()'s pattern.

Copilot · 2026-04-12T02:45:33Z

    Read an agent's recent diary entries. Returns the last N entries
    in chronological order — the agent's personal journal.


The tool_diary_read docstring still describes reading an agent’s personal diary only, but the function signature now supports overriding the target wing. Please update the docstring to mention the wing parameter and clarify how it changes the data source (agent-default vs explicit wing).

Suggested change

Read an agent's recent diary entries. Returns the last N entries

in chronological order — the agent's personal journal.

Read recent diary entries for an agent.

Returns the last N entries from the diary room in chronological order.

If `wing` is omitted or empty, entries are read from the agent's default

personal wing derived from `agent_name`. If `wing` is provided, entries

are read from that explicit wing instead.

Fixed — updated docstring to document wing parameter and how it changes the data source.

Copilot · 2026-04-12T02:45:34Z

+        ~/.claude/projects/-home-<user>-Projects-<project>/session.jsonl
+    We extract <project> as the wing name.  Falls back to "sessions".
+    """
+    match = re.search(r"-Projects-([^/]+?)(?:/|$)", transcript_path)


_wing_from_transcript_path only treats / as a path separator. On Windows (CI runs windows-latest), transcript paths are typically backslash-separated, so this will always fall back to "sessions" and won’t actually fix per-project diary wings. Consider supporting both separators (or use Path(...).parts / normalize separators) when extracting the project segment.

Suggested change

match = re.search(r"-Projects-([^/]+?)(?:/|$)", transcript_path)

normalized_path = transcript_path.replace("\\", "/")

match = re.search(r"-Projects-([^/]+?)(?:/|$)", normalized_path)

Fixed — added path separator normalization (replace("\\\\", "/")) before regex match. Added dedicated tests for Windows backslash paths.

Copilot · 2026-04-12T02:45:34Z

+    assert result["reason"].startswith(STOP_BLOCK_REASON)




The test now only checks reason.startswith(STOP_BLOCK_REASON), which can pass even if the new wing=... suffix is missing or malformed. Add assertions for the expected suffix (e.g. contains wing=sessions for non-Claude paths) and a new case covering a Claude Code transcript path so _wing_from_transcript_path() behavior is exercised.

Suggested change

assert result["reason"].startswith(STOP_BLOCK_REASON)

assert result["reason"].startswith(STOP_BLOCK_REASON)

assert "wing=sessions" in result["reason"]

def test_stop_hook_blocks_at_interval_claude_code_path(tmp_path):

transcript = tmp_path / ".claude" / "projects" / "example" / "t.jsonl"

transcript.parent.mkdir(parents=True, exist_ok=True)

_write_transcript(

transcript,

[{"message": {"role": "user", "content": f"msg {i}"}} for i in range(SAVE_INTERVAL)],

)

result = _capture_hook_output(

hook_stop,

{"session_id": "test", "stop_hook_active": False, "transcript_path": str(transcript)},

state_dir=tmp_path,

)

assert result["decision"] == "block"

assert result["reason"].startswith(STOP_BLOCK_REASON)

assert "wing=projects" in result["reason"]

Fixed — tightened assertion to check for wing=sessions suffix. Added 5 new tests: wing extraction from Claude Code transcript path, project extraction, fallback to sessions, Windows paths, and lowercasing.

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jphein · 2026-04-12T04:34:37Z

Rebased onto current develop (post-#647 and #667). The wing param in diary_read now preserves #647's sanitize_name() validation and last_n clamping — both combined cleanly with the wing parameter logic.

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fork-direction.md and TODO-fork-improvements.md were scattered strategic thinking that belongs in the front-door README: competitive landscape, roadmap (P0-P6), and open problems. Merges them in and deletes the standalone files. PR-status tables (README + CLAUDE.md) were stale after today's rebases of MemPalace#659, MemPalace#660, MemPalace#661, MemPalace#673, MemPalace#681 — updated to reflect current mergeable state. MemPalace#673 specifically noted as cleanly rebased against MemPalace#863. test_readme_claims.py skips MCP-tool-table and dialect-reference checks when absent — our slimmed fork README doesn't reproduce upstream's tool table structure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qodo-ai-reviewer · 2026-04-17T06:26:05Z

Hi, tool_diary_read() ignores agent_name in its Chroma where filter when wing is provided, so it can return entries written by other agents into the same wing.

Severity: action required | Category: security

How to fix: Filter diary read by agent

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

tool_diary_read() can return other agents’ diary entries when callers pass a custom wing, because the Chroma where filter does not include the agent metadata field.

Issue Context

tool_diary_write() stores diary entries with metadata including agent. tool_diary_read() requires agent_name but currently does not use it in the DB query, relying on default wing naming for isolation.

Fix Focus Areas

mempalace/mcp_server.py[989-1045]

Update the where clause to include { "agent": agent_name } (and keep wing/room filters).

We noticed a couple of other issues in this PR as well — happy to share if helpful.

Found by Qodo code review

jphein · 2026-04-18T16:56:44Z

Friendly ping on review — adds an optional wing parameter to tool_diary_write / tool_diary_read and derives the project wing from the Claude Code transcript path when writing from the stop hook, so per-project sessions don't all collapse into the default session-hook wing. Clean against current develop.

Every remaining row in "Still ahead of upstream" now carries a status so the reader can tell at a glance whether each change is being upstreamed, pending a PR, or deliberately fork-local. Dropped: - "Guard ChromaDB 1.5.x metadata-mismatch segfault" — this row was overstated. The memory file for today's debugging notes that the try-get/except-create pattern is defensive code that never reproduced a specific crash (the actual crashes traced to HNSW drift). Leaving it in "Still ahead" implied an upstream-candidate fix, which it isn't. Code stays in place as defensive, but the README no longer claims it as a fork-ahead feature. Moved to Superseded: - "Stale HNSW mtime detection + mempalace_reconnect" — upstream took a different approach in MemPalace#757. Our broader inode+mtime detection and the mempalace_reconnect MCP tool remain as fork-local convenience; they're just not "ahead of upstream" anymore. Statuses now populated: - Linked PR number for the 7 changes with active upstream PRs (MemPalace#659, MemPalace#660, MemPalace#661, MemPalace#673 with APPROVED note, MemPalace#999, MemPalace#1000, MemPalace#1005). - "PR pending" for 3 items that are good candidates but unfiled: epsilon mtime comparison, max_distance parameter, tool output mining. - "fork-only" for 2 items we keep intentionally without pitching upstream: .blob_seq_ids_migrated marker (narrow), bulk_check_mined (complementary to upstream's MemPalace#784 file-locking). Legend sentence added above the table explains the three status values. 42 README-claim tests pass.

bensig · 2026-04-19T06:20:33Z

Code review

Found 2 issues:

_wing_from_transcript_path() returns bare project names (e.g., "myproject") without the wing_ prefix, but all existing wings follow the wing_* convention (documented in AAAK_SPEC: "WINGS: wing_user, wing_agent, wing_team, wing_code, wing_myproject..."). Diary entries written via the stop hook land in wing "myproject" while diary_read defaults to "wing_claude" -- entries become unreachable by default reads.

mempalace/mempalace/hooks_cli.py

Lines 214 to 217 in ca5b357

    
           match = re.search(r"-Projects-([^/]+?)(?:/|$)", normalized) 
        
           if match: 
        
               return match.group(1).lower().replace(" ", "_") 
        
           return "sessions"

tool_diary_read() does not include agent_name in the ChromaDB where filter when a custom wing is provided. The filter is {"$and": [{"wing": wing}, {"room": "diary"}]} with no {"agent": agent_name} clause. Any caller passing a custom wing can read diary entries written by other agents into that wing -- a cross-agent information leak. (Also flagged by Qudo, unresolved.)

mempalace/mempalace/mcp_server.py

Lines 1017 to 1019 in ca5b357

    
           ) 
        
           if not results["ids"]:

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

bensig · 2026-04-19T06:26:11Z

@jphein small fix needed: _wing_from_transcript_path() returns bare project names (e.g. myproject) but existing wings all use the wing_ prefix (wing_myproject). Entries written via the stop hook will be unreachable by default diary_read calls. See review comment above for details.

Addresses bensig's 2-issue review on PR MemPalace#659. 1. _wing_from_transcript_path() was returning bare project names (e.g. "myproject") while all existing wings follow the wing_* convention from AAAK_SPEC (wing_user, wing_agent, wing_code, wing_<project>). 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 (7 tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jphein · 2026-04-19T15:26:32Z

Both fixed in 49704af:

_wing_from_transcript_path() now returns wing_<project> (matches AAAK_SPEC) and falls back to wing_sessions. 6 existing tests updated.
tool_diary_read() now includes {"agent": agent_name} in the $and filter — no cross-agent read access when a custom wing is shared.

All 51 hooks_cli + 62 mcp_server tests pass locally.

…emPalace#1036 Overnight/morning: - MemPalace#681, MemPalace#1000, MemPalace#1023 merged — moved from "Still ahead" to "Merged upstream (post-v3.3.1)" - bensig reviewed MemPalace#659 (wing_ prefix + agent filter) and MemPalace#1021 (silent_guard default) — both addressed on their PR branches - MemPalace#673 needed re-rebase after overnight develop merges; done - MemPalace#1036 filed: paginate miner.status(), closes upstream MemPalace#802 and MemPalace#1015 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… 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>

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

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>

…emPalace#1094 rows + MemPalace#659 CLEAN Three open-PR rows were missing (cmd_export, cmd_purge, None-metadata boundary). MemPalace#659 moved from MERGEABLE to CLEAN after the 2026-04-22 rebase onto current develop cleared its stale-merge blocker. Open-PR count: 4 → 7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

Copilot AI review requested due to automatic review settings April 12, 2026 02:42

Copilot started reviewing on behalf of jphein April 12, 2026 02:43 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

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

fix: sanitize wing param, cross-platform paths, tighten test assertions

7c311ee

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jphein force-pushed the fix/diary-wing-param branch from 7c311ee to 36deb35 Compare April 12, 2026 04:20

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

fix: sanitize wing param, cross-platform paths, tighten test assertions

23a349b

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jphein force-pushed the fix/diary-wing-param branch from 36deb35 to 23a349b Compare April 13, 2026 14:22

jphein mentioned this pull request Apr 13, 2026

fix: sync version.py with pyproject.toml and update stale org URLs (#771, #787) #794

Closed

igorls added area/hooks Claude Code hook scripts (Stop, PreCompact, SessionStart) area/mcp MCP server and tools bug Something isn't working labels Apr 14, 2026

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

fix: sanitize wing param, cross-platform paths, tighten test assertions

ca5b357

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jphein force-pushed the fix/diary-wing-param branch from daf072b to ca5b357 Compare April 16, 2026 16:01

jphein and others added 3 commits April 22, 2026 10:22

fix: sanitize wing param, cross-platform paths, tighten test assertions

b944516

Addresses Copilot review feedback on MemPalace#659. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

jphein force-pushed the fix/diary-wing-param branch from 49704af to 1f5c7bf Compare April 22, 2026 17:27

		Read an agent's recent diary entries. Returns the last N entries
		in chronological order — the agent's personal journal.

-    Read an agent's recent diary entries. Returns the last N entries
-    in chronological order — the agent's personal journal.
+    Read recent diary entries for an agent.
+    Returns the last N entries from the diary room in chronological order.
+    If `wing` is omitted or empty, entries are read from the agent's default
+    personal wing derived from `agent_name`. If `wing` is provided, entries
+    are read from that explicit wing instead.

	match = re.search(r"-Projects-([^/]+?)(?:/\|$)", transcript_path)
	normalized_path = transcript_path.replace("\\", "/")
	match = re.search(r"-Projects-([^/]+?)(?:/\|$)", normalized_path)

-    assert result["reason"].startswith(STOP_BLOCK_REASON)
+    assert result["reason"].startswith(STOP_BLOCK_REASON)
+    assert "wing=sessions" in result["reason"]
+def test_stop_hook_blocks_at_interval_claude_code_path(tmp_path):
+    transcript = tmp_path / ".claude" / "projects" / "example" / "t.jsonl"
+    transcript.parent.mkdir(parents=True, exist_ok=True)
+    _write_transcript(
+        transcript,
+        [{"message": {"role": "user", "content": f"msg {i}"}} for i in range(SAVE_INTERVAL)],
+    )
+    result = _capture_hook_output(
+        hook_stop,
+        {"session_id": "test", "stop_hook_active": False, "transcript_path": str(transcript)},
+        state_dir=tmp_path,
+    )
+    assert result["decision"] == "block"
+    assert result["reason"].startswith(STOP_BLOCK_REASON)
+    assert "wing=projects" in result["reason"]

Conversation

jphein commented Apr 12, 2026

Problem

Solution

mcp_server.py

hooks_cli.py

Backwards Compatibility

Test Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

jphein commented Apr 12, 2026

Uh oh!

qodo-ai-reviewer commented Apr 17, 2026

Issue description

Issue Context

Fix Focus Areas

Uh oh!

jphein commented Apr 18, 2026

Uh oh!

bensig commented Apr 19, 2026

Code review

Uh oh!

bensig commented Apr 19, 2026

Uh oh!

jphein commented Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`mcp_server.py`

`hooks_cli.py`