feat(kanban): carry the review handoff summary into the wake turn
`completed` already puts the worker's summary inside the synthetic wake turn, so the woken creator sees what was done. `review_requested` did not: the summary rode the passive ping only, and the wake turn said just "handed off for review", forcing the woken reviewer to re-read the board (and losing the PR link the worker had already written). Reuse the same first-line handoff the `completed` branch builds, so the existing `gateway.kanban.wake.handoff` string renders it — no new locale keys, no change to the passive message.
This commit is contained in:
@@ -613,7 +613,16 @@ class GatewayKanbanWatchersMixin:
|
||||
# first-class review lane. Wake the origin thread.
|
||||
handoff = ""
|
||||
if ev.payload and ev.payload.get("summary"):
|
||||
handoff = f"\n{str(ev.payload['summary'])[:200]}"
|
||||
summary = str(ev.payload["summary"])
|
||||
handoff = f"\n{summary[:200]}"
|
||||
# Carry the worker's handoff into the wake turn
|
||||
# like ``completed`` does: a reviewer woken with
|
||||
# a bare "ready for review" has to re-read the
|
||||
# board to learn what was implemented.
|
||||
lines = summary.strip().splitlines()
|
||||
wake_handoff = (
|
||||
lines[0][:200] if lines else summary[:200]
|
||||
)
|
||||
msg = (
|
||||
f"👀 {board_tag}{tag}Kanban {sub['task_id']} ready for review"
|
||||
f" — {title}{handoff}"
|
||||
|
||||
@@ -640,7 +640,11 @@ def _wake_text(adapter):
|
||||
return getattr(adapter.handled[0], "text", "") or ""
|
||||
|
||||
|
||||
def _review_handoff_task(*, delivery_mode="notify+wake"):
|
||||
def _review_handoff_task(
|
||||
*,
|
||||
delivery_mode="notify+wake",
|
||||
summary="PR ready: https://example.invalid/pr/7\nfull details below",
|
||||
):
|
||||
conn = kb.connect()
|
||||
try:
|
||||
tid = kb.create_task(
|
||||
@@ -660,7 +664,7 @@ def _review_handoff_task(*, delivery_mode="notify+wake"):
|
||||
kb.claim_task(conn, tid)
|
||||
run_id = kb.get_task(conn, tid).current_run_id
|
||||
assert kb.request_review(
|
||||
conn, tid, summary="implementation done", expected_run_id=run_id,
|
||||
conn, tid, summary=summary, expected_run_id=run_id,
|
||||
) is True
|
||||
return tid
|
||||
finally:
|
||||
@@ -668,7 +672,7 @@ def _review_handoff_task(*, delivery_mode="notify+wake"):
|
||||
|
||||
|
||||
def test_review_requested_wakes_the_origin_session(tmp_path, monkeypatch):
|
||||
"""A review handoff wakes the origin on top of the passive ping."""
|
||||
"""A review handoff wakes the origin and carries the worker's summary."""
|
||||
monkeypatch.setenv("HERMES_KANBAN_DB", str(tmp_path / "review-wake.db"))
|
||||
kb.init_db()
|
||||
tid = _review_handoff_task()
|
||||
@@ -679,7 +683,13 @@ def test_review_requested_wakes_the_origin_session(tmp_path, monkeypatch):
|
||||
|
||||
assert len(adapter.sent) == 1, "the passive review ping is unchanged"
|
||||
assert "ready for review" in adapter.sent[0]["text"]
|
||||
assert tid in _wake_text(adapter)
|
||||
|
||||
wake = _wake_text(adapter)
|
||||
assert tid in wake
|
||||
assert "PR ready: https://example.invalid/pr/7" in wake, (
|
||||
"the worker's handoff must ride the wake turn like it does for "
|
||||
"`completed`, otherwise the woken reviewer has to re-read the board"
|
||||
)
|
||||
|
||||
|
||||
def test_block_loop_detected_wakes_the_origin_session(tmp_path, monkeypatch):
|
||||
|
||||
@@ -1035,7 +1035,7 @@ A subscription removes itself automatically once the task reaches `done` or `arc
|
||||
|
||||
A "wake" forges a synthetic inbound message to the destination gateway agent so it takes a normal turn (reads the comment + result, reasons, replies) instead of getting a one-line passive notification. It only fires when the notifier runs inside a live gateway process; otherwise a `notify+wake` subscription still delivers its passive message, while a `wake`-only subscription does nothing in that process.
|
||||
|
||||
**Which events wake.** The ones that hand a decision back to the origin: `completed`, `blocked`, `gave_up`, `crashed`, `timed_out`, `review_requested` (a worker finished the implementation and handed off via `kanban_request_review`) and `block_loop_detected` (the task was routed to `triage` after repeated blocks). `status`, `archived` and `unblocked` are delivered but never wake — they are bookkeeping transitions, not decisions.
|
||||
**Which events wake.** The ones that hand a decision back to the origin: `completed`, `blocked`, `gave_up`, `crashed`, `timed_out`, `review_requested` (a worker finished the implementation and handed off via `kanban_request_review`) and `block_loop_detected` (the task was routed to `triage` after repeated blocks). `status`, `archived` and `unblocked` are delivered but never wake — they are bookkeeping transitions, not decisions. When a `completed` or `review_requested` event carries a summary, that handoff rides the wake turn, so the woken agent sees what the worker actually did.
|
||||
|
||||
`--chat-type` (`dm` | `group` | `channel` | `thread`) records the originating chat's type so a woken turn resolves the operator's **real** session: `build_session_key` keys groups, channels, and threads differently from DMs, so an inaccurate `chat_type` would route the wake into a separate, context-less session. The `/kanban` auto-subscribe and slash-command paths capture this automatically — you only set it by hand when subscribing a chat from a script or cron. Omit it to leave an existing subscription unchanged (new subscriptions default to `dm`).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user