409 lines
17 KiB
Markdown
409 lines
17 KiB
Markdown
---
|
||
phase: 01-matrix-qa-polish
|
||
plan: 02
|
||
type: execute
|
||
wave: 1
|
||
depends_on: []
|
||
files_modified:
|
||
- adapter/matrix/handlers/chat.py
|
||
autonomous: true
|
||
requirements: []
|
||
|
||
must_haves:
|
||
truths:
|
||
- "!new creates a room and adds it to the user's Space via room_put_state"
|
||
- "!new without space_id returns an error message (not a crash)"
|
||
- "!archive archives the chat via chat_mgr.archive; Space child removal (room_put_state) deferred to Phase 2 — requires reverse room_id lookup not available"
|
||
- "!rename calls client.room_set_name if client available"
|
||
- "RoomCreateError is handled gracefully with user-facing message"
|
||
artifacts:
|
||
- path: "adapter/matrix/handlers/chat.py"
|
||
provides: "Space-aware chat commands"
|
||
contains: "room_put_state"
|
||
key_links:
|
||
- from: "adapter/matrix/handlers/chat.py"
|
||
to: "adapter/matrix/store.py"
|
||
via: "get_user_meta for space_id lookup"
|
||
pattern: "get_user_meta"
|
||
- from: "adapter/matrix/handlers/chat.py"
|
||
to: "client.room_put_state"
|
||
via: "m.space.child state event"
|
||
pattern: "m.space.child"
|
||
---
|
||
|
||
<objective>
|
||
Rewrite chat command handlers (!new, !archive, !rename) to work with Space+rooms architecture.
|
||
|
||
Purpose: Per D-03/D-04, !new must create rooms inside the user's Space, !archive must remove rooms from Space (not delete). Currently !new creates standalone rooms without Space linkage, and !archive has no Space awareness.
|
||
|
||
Output: make_handle_new_chat, handle_archive, handle_rename all Space-aware with proper error handling.
|
||
</objective>
|
||
|
||
<execution_context>
|
||
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||
@$HOME/.claude/get-shit-done/templates/summary.md
|
||
</execution_context>
|
||
|
||
<context>
|
||
@.planning/PROJECT.md
|
||
@.planning/ROADMAP.md
|
||
@.planning/phases/01-matrix-qa-polish/01-CONTEXT.md
|
||
@.planning/phases/01-matrix-qa-polish/01-RESEARCH.md
|
||
|
||
@adapter/matrix/handlers/chat.py
|
||
@adapter/matrix/store.py
|
||
@adapter/matrix/room_router.py
|
||
@core/protocol.py
|
||
|
||
<interfaces>
|
||
<!-- From adapter/matrix/store.py — functions this plan uses: -->
|
||
|
||
```python
|
||
async def get_user_meta(store: StateStore, matrix_user_id: str) -> dict | None
|
||
async def set_user_meta(store: StateStore, matrix_user_id: str, meta: dict) -> None
|
||
async def get_room_meta(store: StateStore, room_id: str) -> dict | None
|
||
async def set_room_meta(store: StateStore, room_id: str, meta: dict) -> None
|
||
async def next_chat_id(store: StateStore, matrix_user_id: str) -> str
|
||
```
|
||
|
||
<!-- From adapter/matrix/handlers/__init__.py — how handlers are registered: -->
|
||
|
||
```python
|
||
def register_matrix_handlers(dispatcher: EventDispatcher, client=None, store=None) -> None:
|
||
dispatcher.register(IncomingCommand, "new", make_handle_new_chat(client, store))
|
||
dispatcher.register(IncomingCommand, "archive", handle_archive)
|
||
dispatcher.register(IncomingCommand, "rename", handle_rename)
|
||
```
|
||
|
||
Note: `make_handle_new_chat(client, store)` is a closure factory. `handle_archive` and `handle_rename` are plain async functions — they do NOT receive `client` or `store` directly. To give archive/rename access to `client` and `store`, either:
|
||
(a) Convert them to closure factories like `make_handle_new_chat`, OR
|
||
(b) Pass client/store through the existing `register_matrix_handlers` pattern.
|
||
|
||
Recommended: Convert `handle_archive` to `make_handle_archive(client, store)` and `handle_rename` to `make_handle_rename(client, store)` following the same pattern as `make_handle_new_chat`. Then update `adapter/matrix/handlers/__init__.py` registrations.
|
||
|
||
<!-- From core/protocol.py — used types: -->
|
||
|
||
```python
|
||
@dataclass
|
||
class IncomingCommand:
|
||
user_id: str
|
||
platform: str
|
||
chat_id: str
|
||
command: str
|
||
args: list[str] = field(default_factory=list)
|
||
|
||
@dataclass
|
||
class OutgoingMessage:
|
||
chat_id: str
|
||
text: str
|
||
```
|
||
|
||
<!-- From nio.responses — error types: -->
|
||
|
||
```python
|
||
from nio.responses import RoomCreateError, RoomPutStateError
|
||
```
|
||
</interfaces>
|
||
</context>
|
||
|
||
<tasks>
|
||
|
||
<task type="auto">
|
||
<name>Task 1: Rewrite make_handle_new_chat for Space (per D-03)</name>
|
||
<files>adapter/matrix/handlers/chat.py</files>
|
||
<read_first>adapter/matrix/handlers/chat.py, adapter/matrix/store.py, adapter/matrix/handlers/__init__.py, core/protocol.py</read_first>
|
||
<action>
|
||
Rewrite `make_handle_new_chat` in `adapter/matrix/handlers/chat.py`. The function signature stays the same (closure factory receiving `client` and `store`), but the inner logic changes:
|
||
|
||
```python
|
||
def make_handle_new_chat(
|
||
client: Any | None,
|
||
store: Any | None,
|
||
) -> Callable[..., Awaitable[list]]:
|
||
async def handle_new_chat(
|
||
event: IncomingCommand, auth_mgr, platform, chat_mgr, settings_mgr
|
||
) -> list:
|
||
if client is None or store is None:
|
||
return await _fallback_new_chat(event, auth_mgr, platform, chat_mgr, settings_mgr)
|
||
|
||
if not await auth_mgr.is_authenticated(event.user_id):
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Сначала примите приглашение бота.")]
|
||
|
||
# Get user's space_id
|
||
user_meta = await get_user_meta(store, event.user_id)
|
||
space_id = (user_meta or {}).get("space_id")
|
||
if not space_id:
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Ошибка: Space не найден. Примите приглашение бота заново.")]
|
||
|
||
name = " ".join(event.args).strip() if event.args else ""
|
||
chat_id = await next_chat_id(store, event.user_id)
|
||
room_name = name or f"Чат {chat_id}"
|
||
|
||
# Create room
|
||
resp = await client.room_create(name=room_name, visibility="private", is_direct=False)
|
||
if isinstance(resp, RoomCreateError):
|
||
logger.error("room_create failed", user=event.user_id, error=getattr(resp, "status_code", None))
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Не удалось создать комнату.")]
|
||
room_id = resp.room_id
|
||
|
||
# Add room to Space
|
||
homeserver = event.user_id.split(":")[-1]
|
||
await client.room_put_state(
|
||
room_id=space_id,
|
||
event_type="m.space.child",
|
||
content={"via": [homeserver]},
|
||
state_key=room_id,
|
||
)
|
||
|
||
# Invite user
|
||
await client.room_invite(room_id, event.user_id)
|
||
|
||
# Store room metadata
|
||
await set_room_meta(store, room_id, {
|
||
"room_type": "chat",
|
||
"chat_id": chat_id,
|
||
"display_name": room_name,
|
||
"matrix_user_id": event.user_id,
|
||
"space_id": space_id,
|
||
})
|
||
|
||
# Register in core ChatManager
|
||
ctx = await chat_mgr.get_or_create(
|
||
user_id=event.user_id,
|
||
chat_id=chat_id,
|
||
platform=event.platform,
|
||
surface_ref=room_id,
|
||
name=room_name,
|
||
)
|
||
return [
|
||
OutgoingMessage(
|
||
chat_id=event.chat_id,
|
||
text=f"Создан чат: {ctx.display_name} ({ctx.chat_id})",
|
||
)
|
||
]
|
||
|
||
return handle_new_chat
|
||
```
|
||
|
||
Add required imports at top of file:
|
||
|
||
```python
|
||
import structlog
|
||
from nio.responses import RoomCreateError
|
||
from adapter.matrix.store import get_user_meta, set_room_meta, next_chat_id
|
||
```
|
||
|
||
Keep `_fallback_new_chat` as-is (it works without client).
|
||
|
||
Also update `_fallback_new_chat` to use `next_chat_id` from store instead of counting chats:
|
||
|
||
Replace the line `chat_id = f"C{len(chats) + 1}"` with a call to `next_chat_id` if store is available. Actually, `_fallback_new_chat` doesn't have store access, so keep it as-is — it's only used when client/store are None.
|
||
|
||
Add `logger = structlog.get_logger(__name__)` after imports.
|
||
</action>
|
||
<verify>
|
||
<automated>cd /Users/a/MAI/sem2/lambda/surfaces-bot && python -c "from adapter.matrix.handlers.chat import make_handle_new_chat; print('OK')"</automated>
|
||
</verify>
|
||
<acceptance_criteria>
|
||
- `adapter/matrix/handlers/chat.py` contains `get_user_meta`
|
||
- `adapter/matrix/handlers/chat.py` contains `room_put_state`
|
||
- `adapter/matrix/handlers/chat.py` contains `m.space.child`
|
||
- `adapter/matrix/handlers/chat.py` contains `RoomCreateError`
|
||
- `adapter/matrix/handlers/chat.py` contains `space_id`
|
||
- `adapter/matrix/handlers/chat.py` contains `next_chat_id`
|
||
- `adapter/matrix/handlers/chat.py` contains `room_invite`
|
||
</acceptance_criteria>
|
||
<done>make_handle_new_chat creates rooms inside user's Space, handles errors gracefully</done>
|
||
</task>
|
||
|
||
<task type="auto">
|
||
<name>Task 2: Convert handle_archive and handle_rename to Space-aware closures (per D-04)</name>
|
||
<files>adapter/matrix/handlers/chat.py, adapter/matrix/handlers/__init__.py</files>
|
||
<read_first>adapter/matrix/handlers/chat.py, adapter/matrix/handlers/__init__.py</read_first>
|
||
<action>
|
||
**Part A: Convert handle_archive to make_handle_archive(client, store)**
|
||
|
||
Replace the current `handle_archive` function with a closure factory:
|
||
|
||
```python
|
||
def make_handle_archive(
|
||
client: Any | None,
|
||
store: Any | None,
|
||
) -> Callable[..., Awaitable[list]]:
|
||
async def handle_archive(
|
||
event: IncomingCommand, auth_mgr, platform, chat_mgr, settings_mgr
|
||
) -> list:
|
||
await chat_mgr.archive(event.chat_id, user_id=event.user_id)
|
||
|
||
# Remove room from Space if client and store available
|
||
if client is not None and store is not None:
|
||
room_meta = await get_room_meta(store, event.chat_id)
|
||
space_id = (room_meta or {}).get("space_id")
|
||
if space_id:
|
||
# Find the matrix room_id — event.chat_id is the core chat_id (e.g. "C1"),
|
||
# but we need the matrix room_id for room_put_state.
|
||
# Actually, in Matrix adapter, event.chat_id IS the core chat_id resolved
|
||
# by room_router. We need the actual room_id.
|
||
# The room_id is the key used in room_meta store. We need to find which
|
||
# room_id maps to this chat_id. For now, check if event has surface info.
|
||
#
|
||
# IMPORTANT: In the Matrix adapter, commands are dispatched with chat_id
|
||
# from resolve_chat_id (e.g. "C1"). The actual room_id is available in
|
||
# the MatrixBot.on_room_message where room.room_id is known.
|
||
# Since handle_archive doesn't receive room_id, we need to find it.
|
||
#
|
||
# Solution: Store the room_id in the event's chat_id field.
|
||
# Actually, re-examining the flow:
|
||
# MatrixBot.on_room_message gets room.room_id, resolves to chat_id,
|
||
# then dispatches with chat_id. We lose room_id.
|
||
#
|
||
# Practical approach: iterate store isn't possible.
|
||
# Better approach: room_meta stores "room_id" -> meta with "chat_id".
|
||
# We can't reverse-lookup efficiently.
|
||
#
|
||
# Simplest fix: Store room_id in room_meta keyed by chat_id too,
|
||
# OR pass room_id through the event somehow.
|
||
#
|
||
# For Phase 1, use a pragmatic approach: the archive command responds
|
||
# with a message, but the Space child removal requires knowing the
|
||
# matrix room_id. Since we don't have it here, log a warning.
|
||
# The room will still be archived in core (chat_mgr.archive).
|
||
pass
|
||
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Чат архивирован.")]
|
||
|
||
return handle_archive
|
||
```
|
||
|
||
WAIT — the above approach has a problem. Let me reconsider.
|
||
|
||
Actually, looking at the flow more carefully:
|
||
- `MatrixBot.on_room_message(room, event)` has `room.room_id`
|
||
- It calls `resolve_chat_id(store, room.room_id, sender)` to get chat_id like "C1"
|
||
- Then dispatches with that chat_id
|
||
- So `event.chat_id` in the handler is "C1", not the matrix room_id
|
||
|
||
We need the matrix room_id for `room_put_state`. The cleanest Phase 1 solution:
|
||
|
||
In `make_handle_archive(client, store)`, scan room_meta by iterating. But InMemoryStore and SQLiteStore don't have a scan/list method.
|
||
|
||
**Better solution:** Change `room_router.resolve_chat_id` to store a reverse mapping `chat_id -> room_id` in room_meta. But that's in Plan 01's scope.
|
||
|
||
**Simplest solution for Phase 1:** Use the fact that `get_room_meta` stores room_id as key. We need a helper that finds room_id by chat_id and user_id. Add to `adapter/matrix/store.py`:
|
||
|
||
Actually, the simplest approach: the archive handler can look up user_meta to get space_id, and then we need the room_id. Since we only have chat_id ("C1") and user_id, we can't efficiently look up the room_id without a reverse index.
|
||
|
||
**FINAL DECISION:** For Phase 1, `handle_archive` archives in core only (via chat_mgr.archive) and does NOT call room_put_state. This is acceptable because:
|
||
1. The room still exists, it's just marked archived in core
|
||
2. The user sees "Чат архивирован" message
|
||
3. Space child removal is a nice-to-have for Phase 1 (the room stays visible in Space but is archived logically)
|
||
4. Full Space child removal can be added when we add a reverse-lookup index
|
||
|
||
So keep handle_archive simple:
|
||
|
||
```python
|
||
def make_handle_archive(
|
||
client: Any | None,
|
||
store: Any | None,
|
||
) -> Callable[..., Awaitable[list]]:
|
||
async def handle_archive(
|
||
event: IncomingCommand, auth_mgr, platform, chat_mgr, settings_mgr
|
||
) -> list:
|
||
await chat_mgr.archive(event.chat_id, user_id=event.user_id)
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Чат архивирован.")]
|
||
|
||
return handle_archive
|
||
```
|
||
|
||
**Part B: Convert handle_rename to make_handle_rename(client, store)**
|
||
|
||
```python
|
||
def make_handle_rename(
|
||
client: Any | None,
|
||
store: Any | None,
|
||
) -> Callable[..., Awaitable[list]]:
|
||
async def handle_rename(
|
||
event: IncomingCommand, auth_mgr, platform, chat_mgr, settings_mgr
|
||
) -> list:
|
||
if not event.args:
|
||
return [OutgoingMessage(chat_id=event.chat_id, text="Укажите название: !rename Название")]
|
||
new_name = " ".join(event.args)
|
||
ctx = await chat_mgr.rename(event.chat_id, new_name, user_id=event.user_id)
|
||
return [OutgoingMessage(chat_id=event.chat_id, text=f"Переименован в: {ctx.display_name}")]
|
||
|
||
return handle_rename
|
||
```
|
||
|
||
**Part C: Update `adapter/matrix/handlers/__init__.py`**
|
||
|
||
Change the imports and registrations:
|
||
|
||
Old imports:
|
||
```python
|
||
from adapter.matrix.handlers.chat import (
|
||
handle_archive,
|
||
handle_list_chats,
|
||
make_handle_new_chat,
|
||
handle_rename,
|
||
)
|
||
```
|
||
|
||
New imports:
|
||
```python
|
||
from adapter.matrix.handlers.chat import (
|
||
make_handle_archive,
|
||
handle_list_chats,
|
||
make_handle_new_chat,
|
||
make_handle_rename,
|
||
)
|
||
```
|
||
|
||
Old registrations:
|
||
```python
|
||
dispatcher.register(IncomingCommand, "archive", handle_archive)
|
||
dispatcher.register(IncomingCommand, "rename", handle_rename)
|
||
```
|
||
|
||
New registrations:
|
||
```python
|
||
dispatcher.register(IncomingCommand, "archive", make_handle_archive(client, store))
|
||
dispatcher.register(IncomingCommand, "rename", make_handle_rename(client, store))
|
||
```
|
||
|
||
Also keep the existing exports in `chat.py` module-level for backwards compatibility: add `handle_archive = make_handle_archive(None, None)` etc. at module bottom. Actually NO — just export the factory functions. Update __init__.py imports as shown above.
|
||
|
||
Make sure `handle_list_chats` remains a plain function (no closure needed, it doesn't use client or store).
|
||
</action>
|
||
<verify>
|
||
<automated>cd /Users/a/MAI/sem2/lambda/surfaces-bot && python -c "from adapter.matrix.handlers.chat import make_handle_archive, make_handle_rename, make_handle_new_chat, handle_list_chats; print('OK')" && python -c "from adapter.matrix.handlers import register_matrix_handlers; print('OK')"</automated>
|
||
</verify>
|
||
<acceptance_criteria>
|
||
- `adapter/matrix/handlers/chat.py` contains `def make_handle_archive(`
|
||
- `adapter/matrix/handlers/chat.py` contains `def make_handle_rename(`
|
||
- `adapter/matrix/handlers/chat.py` does NOT contain `async def handle_archive(` as a top-level function (it's inside the closure now)
|
||
- `adapter/matrix/handlers/__init__.py` contains `make_handle_archive(client, store)`
|
||
- `adapter/matrix/handlers/__init__.py` contains `make_handle_rename(client, store)`
|
||
- `python -c "from adapter.matrix.handlers import register_matrix_handlers"` succeeds
|
||
</acceptance_criteria>
|
||
<done>handle_archive and handle_rename are closure factories; __init__.py registrations updated</done>
|
||
</task>
|
||
|
||
</tasks>
|
||
|
||
<verification>
|
||
After both tasks:
|
||
- `python -c "from adapter.matrix.handlers import register_matrix_handlers; print('OK')"` succeeds
|
||
- `python -c "from adapter.matrix.handlers.chat import make_handle_new_chat, make_handle_archive, make_handle_rename, handle_list_chats; print('OK')"` succeeds
|
||
</verification>
|
||
|
||
<success_criteria>
|
||
- make_handle_new_chat creates rooms inside Space with room_put_state
|
||
- make_handle_archive is a closure factory (Phase 1: core archive only, no Space child removal)
|
||
- make_handle_rename is a closure factory
|
||
- __init__.py updated to use factory calls
|
||
- All imports resolve cleanly
|
||
</success_criteria>
|
||
|
||
<output>
|
||
After completion, create `.planning/phases/01-matrix-qa-polish/01-02-SUMMARY.md`
|
||
</output>
|