Mikhail Putilovskij
6ced154124
- harden Matrix onboarding/chat lifecycle after manual QA - refresh README and Matrix docs to match current behavior - add local ignores for runtime artifacts and include current planning/report docs Closes #7 Closes #9 Closes #14
250 lines
13 KiB
Markdown
250 lines
13 KiB
Markdown
---
|
|
phase: 01-matrix-qa-polish
|
|
plan: 05
|
|
type: execute
|
|
wave: 1
|
|
depends_on: []
|
|
files_modified:
|
|
- adapter/matrix/bot.py
|
|
- adapter/matrix/converter.py
|
|
- adapter/matrix/handlers/confirm.py
|
|
- adapter/matrix/store.py
|
|
- tests/adapter/matrix/test_converter.py
|
|
- tests/adapter/matrix/test_confirm.py
|
|
- tests/adapter/matrix/test_send_outgoing.py
|
|
autonomous: true
|
|
gap_closure: true
|
|
requirements: []
|
|
|
|
must_haves:
|
|
truths:
|
|
- "A Matrix user can confirm an action in the same room where Lambda requested confirmation, even when the logical chat id differs from the Matrix room id."
|
|
- "A Matrix user can cancel an action in the same room where Lambda requested confirmation without affecting another user's pending state."
|
|
- "Confirmation state survives the Matrix adapter send/receive round trip using D-08's `(user_id, room_id)` scope."
|
|
artifacts:
|
|
- path: "adapter/matrix/store.py"
|
|
provides: "Pending-confirm helpers keyed by Matrix user id plus room id."
|
|
- path: "adapter/matrix/converter.py"
|
|
provides: "Command callback payloads that retain Matrix room context."
|
|
- path: "adapter/matrix/handlers/confirm.py"
|
|
provides: "User-and-room-aware confirm and cancel handlers."
|
|
- path: "tests/adapter/matrix/test_send_outgoing.py"
|
|
provides: "Adapter-level send_outgoing -> !yes/!no regression coverage."
|
|
key_links:
|
|
- from: "adapter/matrix/bot.py"
|
|
to: "adapter/matrix/handlers/confirm.py"
|
|
via: "pending_confirm keyed by Matrix user id plus room id, with room_id carried through IncomingCallback payload"
|
|
pattern: "matrix_user_id|room_id"
|
|
- from: "tests/adapter/matrix/test_send_outgoing.py"
|
|
to: "adapter/matrix/bot.py"
|
|
via: "send_outgoing stores pending state before confirm handler resolves it"
|
|
pattern: "set_pending_confirm|make_handle_confirm|make_handle_cancel"
|
|
---
|
|
|
|
<objective>
|
|
Close the blocker where Matrix `send_outgoing` and the runtime `!yes` / `!no` path do not agree on the D-08 confirmation scope.
|
|
|
|
Purpose: Per D-06/D-08 and the verification blocker, Phase 01 is not complete until the text-confirmation flow works end-to-end in the real adapter path using confirmation state scoped per `(user_id, room_id)`, not only in unit tests seeded with `C1`.
|
|
Output: A user-and-room-aware callback contract across `send_outgoing`, command conversion, store helpers, and confirm handlers, plus regression tests that exercise `OutgoingUI` -> `!yes` / `!no`.
|
|
</objective>
|
|
|
|
<execution_context>
|
|
@/Users/a/.codex/get-shit-done/workflows/execute-plan.md
|
|
@/Users/a/.codex/get-shit-done/templates/summary.md
|
|
</execution_context>
|
|
|
|
<context>
|
|
@.planning/STATE.md
|
|
@.planning/ROADMAP.md
|
|
@.planning/phases/01-matrix-qa-polish/01-CONTEXT.md
|
|
@.planning/phases/01-matrix-qa-polish/01-RESEARCH.md
|
|
@.planning/phases/01-matrix-qa-polish/01-VERIFICATION.md
|
|
@.planning/phases/01-matrix-qa-polish/01-03-SUMMARY.md
|
|
@.planning/phases/01-matrix-qa-polish/01-04-SUMMARY.md
|
|
@adapter/matrix/bot.py
|
|
@adapter/matrix/converter.py
|
|
@adapter/matrix/handlers/confirm.py
|
|
@adapter/matrix/store.py
|
|
@tests/adapter/matrix/test_confirm.py
|
|
@tests/adapter/matrix/test_send_outgoing.py
|
|
|
|
<interfaces>
|
|
From `adapter/matrix/bot.py`:
|
|
|
|
```python
|
|
async def send_outgoing(
|
|
client: AsyncClient,
|
|
room_id: str,
|
|
event: OutgoingEvent,
|
|
store: StateStore | None = None,
|
|
) -> None
|
|
```
|
|
|
|
From `adapter/matrix/store.py`:
|
|
|
|
```python
|
|
async def get_room_meta(store: StateStore, room_id: str) -> dict | None
|
|
async def get_pending_confirm(...) -> dict | None
|
|
async def set_pending_confirm(...) -> None
|
|
async def clear_pending_confirm(...) -> None
|
|
```
|
|
|
|
From `adapter/matrix/converter.py`:
|
|
|
|
```python
|
|
def from_command(body: str, sender: str, chat_id: str) -> IncomingEvent
|
|
def from_room_event(event: Any, room_id: str, chat_id: str) -> IncomingEvent | None
|
|
```
|
|
|
|
From `core/protocol.py`:
|
|
|
|
```python
|
|
@dataclass
|
|
class IncomingCallback:
|
|
user_id: str
|
|
platform: str
|
|
chat_id: str
|
|
action: str
|
|
payload: dict[str, Any] = field(default_factory=dict)
|
|
```
|
|
</interfaces>
|
|
</context>
|
|
|
|
<tasks>
|
|
|
|
<task type="auto">
|
|
<name>Task 1: Preserve Matrix user-and-room identity through the `!yes` / `!no` callback path</name>
|
|
<files>adapter/matrix/converter.py, adapter/matrix/handlers/confirm.py, adapter/matrix/bot.py, adapter/matrix/store.py</files>
|
|
<read_first>adapter/matrix/converter.py, adapter/matrix/handlers/confirm.py, adapter/matrix/bot.py, adapter/matrix/store.py, .planning/phases/01-matrix-qa-polish/01-VERIFICATION.md</read_first>
|
|
<behavior>
|
|
- Test 1: `from_room_event(..., room_id=\"!room:example\", chat_id=\"C7\")` for `!yes` or `!no` preserves the core `chat_id` and adds `payload["room_id"] == "!room:example"`.
|
|
- Test 2: `send_outgoing` derives the Matrix user dimension from stored room metadata such as `room_meta["matrix_user_id"]` and persists confirmation state under `(user_id, room_id)`.
|
|
- Test 3: `make_handle_confirm` and `make_handle_cancel` resolve pending state by `(event.user_id, payload["room_id"])`, so a stored confirmation under `("@alice:example.org", "!room:example")` is found even when `event.chat_id` is `C7`.
|
|
- Test 4: If a legacy caller does not provide `payload["room_id"]`, handlers keep the current fallback behavior instead of crashing, while the Matrix adapter path uses the D-08 composite key.
|
|
</behavior>
|
|
<action>
|
|
Implement a single stable `(user_id, room_id)` key across the runtime flow per D-08. Update the Matrix pending-confirm store helpers to accept both `user_id` and `room_id`. Update `from_command` / `from_room_event` so Matrix command callbacks carry the originating `room_id` in `IncomingCallback.payload`. Update `send_outgoing` to derive the user dimension before persisting confirmation state; use stored room metadata such as `get_room_meta(store, room_id)["matrix_user_id"]` because `send_outgoing` currently receives only `room_id`, not `user_id`. Update `make_handle_confirm` and `make_handle_cancel` to read and clear pending confirmations by `(event.user_id, payload["room_id"])` first, with a compatibility fallback only where needed for non-Matrix or older tests.
|
|
|
|
Do not widen this task into protocol changes, new core event types, or reaction support restoration. The only contract change should be the Matrix adapter adding room context into callback payloads and consuming the D-08 composite key consistently.
|
|
</action>
|
|
<verify>
|
|
<automated>cd /Users/a/MAI/sem2/lambda/surfaces-bot && python - <<'PY'
|
|
from types import SimpleNamespace
|
|
|
|
from adapter.matrix.bot import send_outgoing
|
|
from adapter.matrix.converter import from_room_event
|
|
from adapter.matrix.handlers.confirm import make_handle_confirm
|
|
from adapter.matrix.store import get_pending_confirm, set_room_meta
|
|
from core.auth import AuthManager
|
|
from core.chat import ChatManager
|
|
from core.protocol import IncomingCallback, OutgoingUI, UIButton
|
|
from core.settings import SettingsManager
|
|
from core.store import InMemoryStore
|
|
from sdk.mock import MockPlatformClient
|
|
|
|
|
|
async def main():
|
|
callback = from_room_event(
|
|
SimpleNamespace(
|
|
sender="@alice:example.org",
|
|
body="!yes",
|
|
event_id="$e1",
|
|
msgtype="m.text",
|
|
replyto_event_id=None,
|
|
),
|
|
room_id="!room:example.org",
|
|
chat_id="C7",
|
|
)
|
|
assert isinstance(callback, IncomingCallback)
|
|
assert callback.chat_id == "C7"
|
|
assert callback.payload["room_id"] == "!room:example.org"
|
|
|
|
store = InMemoryStore()
|
|
await set_room_meta(
|
|
store,
|
|
"!room:example.org",
|
|
{"matrix_user_id": "@alice:example.org", "chat_id": "C7", "space_id": "!space:example.org"},
|
|
)
|
|
platform = MockPlatformClient()
|
|
chat_mgr = ChatManager(platform, store)
|
|
auth_mgr = AuthManager(platform, store)
|
|
settings_mgr = SettingsManager(platform, store)
|
|
async def room_send(*args, **kwargs):
|
|
return None
|
|
client = SimpleNamespace(room_send=room_send)
|
|
await send_outgoing(
|
|
client,
|
|
"!room:example.org",
|
|
OutgoingUI(
|
|
chat_id="C7",
|
|
text="Archive room",
|
|
buttons=[UIButton(label="Confirm", action="archive", payload={})],
|
|
),
|
|
store=store,
|
|
)
|
|
pending = await get_pending_confirm(store, "@alice:example.org", "!room:example.org")
|
|
assert pending is not None
|
|
handler = make_handle_confirm(store)
|
|
result = await handler(callback, auth_mgr, platform, chat_mgr, settings_mgr)
|
|
assert "Archive room" in result[0].text
|
|
assert await get_pending_confirm(store, "@alice:example.org", "!room:example.org") is None
|
|
|
|
|
|
import asyncio
|
|
asyncio.run(main())
|
|
print("OK")
|
|
PY</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- `adapter/matrix/converter.py` passes the Matrix `room_id` into `IncomingCallback.payload` for `!yes` and `!no`.
|
|
- `adapter/matrix/store.py` exposes pending-confirm helpers keyed by both `user_id` and `room_id`.
|
|
- `adapter/matrix/handlers/confirm.py` uses `(event.user_id, Matrix room_id)` as the primary pending-confirm lookup key.
|
|
- `adapter/matrix/bot.py` derives the Matrix user dimension from stored room metadata before persisting pending confirmations.
|
|
- No code path reintroduces reaction callbacks or room-only/chat-id-only persistence for Matrix confirmations on the Matrix adapter path.
|
|
</acceptance_criteria>
|
|
<done>Matrix confirmation state is keyed consistently across send, confirm, and cancel runtime flow using the D-08 `(user_id, room_id)` scope.</done>
|
|
</task>
|
|
|
|
<task type="auto" tdd="true">
|
|
<name>Task 2: Add end-to-end adapter regression tests for `send_outgoing` -> `!yes` / `!no`</name>
|
|
<files>tests/adapter/matrix/test_converter.py, tests/adapter/matrix/test_send_outgoing.py, tests/adapter/matrix/test_confirm.py</files>
|
|
<read_first>tests/adapter/matrix/test_converter.py, tests/adapter/matrix/test_send_outgoing.py, tests/adapter/matrix/test_confirm.py, adapter/matrix/bot.py, adapter/matrix/converter.py, adapter/matrix/handlers/confirm.py</read_first>
|
|
<behavior>
|
|
- Test 1: `test_converter.py` asserts that Matrix `!yes` / `!no` callbacks preserve `chat_id` but also carry `payload["room_id"]`.
|
|
- Test 2: Sending an `OutgoingUI` with buttons stores pending confirmation under `(user_id, room_id)`, then a converted `!yes` callback resolves it and clears the store for that user in that room.
|
|
- Test 3: The same setup followed by `!no` clears the store and returns the cancellation message for that user in that room.
|
|
- Test 4: The regression tests use distinct room ids and core chat ids so they fail if the implementation falls back to brittle `C1` assumptions.
|
|
</behavior>
|
|
<action>
|
|
Extend the Matrix regression suite with adapter-level tests that exercise the real Phase 01 flow instead of seeding store state directly under `C1`. Add explicit converter assertions in `tests/adapter/matrix/test_converter.py` for `payload["room_id"]`, then use `send_outgoing(...)` to create the pending confirmation, `from_room_event(...)` to convert `!yes` / `!no` from a real Matrix room event, and `make_handle_confirm` / `make_handle_cancel` to resolve the callback. Seed the tests with mismatched values such as `room_id="!confirm:example.org"` and `chat_id="C7"` so the regression proves room-based behavior. The tests must also prove that storage is scoped by `event.user_id` plus `room_id`, not by room alone.
|
|
|
|
Keep the tests isolated to adapter modules; do not route through unrelated core handlers or introduce brittle mocks of `StateStore`, `ChatManager`, or `SettingsManager`.
|
|
</action>
|
|
<verify>
|
|
<automated>cd /Users/a/MAI/sem2/lambda/surfaces-bot && pytest tests/adapter/matrix/test_converter.py tests/adapter/matrix/test_send_outgoing.py tests/adapter/matrix/test_confirm.py -q</automated>
|
|
</verify>
|
|
<acceptance_criteria>
|
|
- `tests/adapter/matrix/test_converter.py` contains explicit assertions for `payload["room_id"]` on Matrix `!yes` / `!no`.
|
|
- `tests/adapter/matrix/test_send_outgoing.py` contains at least one regression test covering `OutgoingUI` -> `!yes` with pending state stored under `(user_id, room_id)`.
|
|
- `tests/adapter/matrix/test_send_outgoing.py` contains at least one regression test covering `OutgoingUI` -> `!no` with pending state stored under `(user_id, room_id)`.
|
|
- `tests/adapter/matrix/test_confirm.py` no longer seeds or asserts the primary confirmation path under hardcoded `C1`.
|
|
- The new tests fail if `payload["room_id"]` is dropped from Matrix command conversion.
|
|
</acceptance_criteria>
|
|
<done>The Matrix suite contains a true adapter-level confirmation regression that covers both confirm and cancel commands under the D-08 user-and-room scope.</done>
|
|
</task>
|
|
|
|
</tasks>
|
|
|
|
<verification>
|
|
Run `pytest tests/adapter/matrix/test_converter.py tests/adapter/matrix/test_send_outgoing.py tests/adapter/matrix/test_confirm.py -q` and confirm the converter and both user-and-room-scoped regression paths pass.
|
|
</verification>
|
|
|
|
<success_criteria>
|
|
- `send_outgoing` -> `!yes` resolves a stored confirmation for the same Matrix user in the same Matrix room.
|
|
- `send_outgoing` -> `!no` clears a stored confirmation for the same Matrix user in the same Matrix room.
|
|
- The adapter path no longer drifts away from D-08's `(user_id, room_id)` confirmation scope.
|
|
</success_criteria>
|
|
|
|
<output>
|
|
After completion, create `.planning/phases/01-matrix-qa-polish/01-05-SUMMARY.md`
|
|
</output>
|