feat(mcp): expose list_hallways and delete_hallway tools

[ggettert]

What does this PR do?

Closes #1739.

Wires the existing hallways.list_hallways and hallways.delete_hallway Python API into the MCP tool registry, mirroring the existing tunnel-tool pattern in mcp_server.py.

After 3.3.6 (#1558), hallways were being produced at mine time but were not retrievable through MCP — list_tunnels / find_tunnels / follow_tunnels / create_tunnel / delete_tunnel all had MCP wiring, hallways had none.

Two new tools:

• mempalace_list_hallways(wing: str | None = None) — wraps hallways.list_hallways. Optional wing filter routes through _sanitize_optional_name the same way tool_list_tunnels does, so invalid names surface a structured error rather than crashing.
• mempalace_delete_hallway(hallway_id: str) — wraps hallways.delete_hallway and returns {"deleted": bool} so callers can distinguish a successful delete from a no-op without re-reading the hallway list.

Pure pass-throughs of the existing Python API. No behavior change in the underlying hallway storage, no schema change to hallways.json.

How to test

uv run pytest tests/test_mcp_server.py -k hallway -v

Should report 7 passed. Full file (uv run pytest tests/test_mcp_server.py) reports 188 passed.

Tests added in TestWriteTools (mirroring where the tunnel-tool tests live):

• test_tool_list_hallways_returns_all_without_filter
• test_tool_list_hallways_filters_by_wing
• test_tool_list_hallways_rejects_invalid_wing_name
• test_tool_delete_hallway_removes_existing_record
• test_tool_delete_hallway_unknown_id_returns_false
• test_tool_delete_hallway_requires_string_id
• test_hallway_tools_registered_in_tools_registry

All seven use monkeypatch to point hallways._HALLWAY_FILE at a tmp_path location so the real ~/.mempalace/hallways.json is never touched.

Checklist

• Tests pass (uv run pytest tests/ -v) — 7 new + 188 in test_mcp_server.py
• No hardcoded paths
• Linter passes (uv run ruff check . and ruff format --check)

---

Verified against develop HEAD (4ceb880, 2026-06-08).

[gemini-code-assist]

Code Review

This pull request introduces hallway MCP tools (mempalace_list_hallways and mempalace_delete_hallway) to list and delete within-wing hallway records, along with comprehensive unit tests. Feedback was provided to add a type check for the wing parameter in tool_list_hallways to prevent potential AttributeError crashes when non-string arguments are passed.

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.

[gemini-code-assist]

If wing is passed as a non-string (e.g., an integer or boolean) by an MCP client, _sanitize_optional_name will attempt to call .strip() on it, raising an unhandled AttributeError. This error will bubble up as an internal server error (code -32000) instead of a clean validation error. Adding a type check ensures robust defensive programming and returns a structured error response.

Suggested change

	def tool_list_hallways(wing: str = None):
	"""List within-wing hallway records, optionally filtered by wing."""
	try:
	wing = _sanitize_optional_name(wing, "wing")
	except ValueError as e:
	return {"error": str(e)}
	return list_hallways(wing)
	def tool_list_hallways(wing: str = None):
	"""List within-wing hallway records, optionally filtered by wing."""
	if wing is not None and not isinstance(wing, str):
	return {"error": "wing must be a string"}
	try:
	wing = _sanitize_optional_name(wing, "wing")
	except ValueError as e:
	return {"error": str(e)}
	return list_hallways(wing)

[ggettert]

Going to push back on this one.

The crash path Gemini flags exists identically in tool_list_tunnels (which this PR mirrored exactly) and in the other read-tool callers of _sanitize_optional_name (e.g. tool_list_drawers at line 1156). It's not specific to this PR — it's a property of the shared _sanitize_optional_name helper.

The repo also has a stated convention in TestParamShapeDiagnostics (line 3345 of tests/test_mcp_server.py):

Dispatch-level TypeError on tools/call should surface as JSON-RPC -32602 (Invalid params) with the offending parameter named. Handler-internal TypeError and non-TypeError exceptions stay generic -32000 (no internals leak).

The AttributeError raised by _sanitize_optional_name on a non-string input is a handler-internal exception under that contract, so it correctly surfaces as -32000. Adding the type-check would change the contract for these two tools while leaving the rest of the read-tool family unchanged, which seems worse than the status quo.