docs(01): create phase plan — 4 plans across 3 waves
This commit is contained in:
parent
a433a2c231
commit
d2a6709f22
5 changed files with 2163 additions and 5 deletions
409
.planning/phases/01-matrix-qa-polish/01-02-PLAN.md
Normal file
409
.planning/phases/01-matrix-qa-polish/01-02-PLAN.md
Normal file
|
|
@ -0,0 +1,409 @@
|
|||
---
|
||||
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 removes room from Space via room_put_state with empty content"
|
||||
- "!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>
|
||||
Loading…
Add table
Add a link
Reference in a new issue