[qingyun-wu]

What

Owner's question: "after connecting, does the agent keep watching the doc, or does a person have to ping it?" The honest answer on main is a person has to ping: every CLI command connects, acts and leaves, and the library's only observer serves board re-assertion. An agent "in" a document had an open socket and no way to learn that a new line addressed it.

Now:

• RoomDoc.changes() — yields the text after every remote edit (the agent's own writes are not news) and raises the close reason when the session ends, so a watcher that stopped is distinguishable from one that saw nothing.
• room_doc.py watch <room> --for <handle>… — holds the document open and prints MENTION\t<line> for each new line that @-mentions a handle. Silence means "nothing yet", never "not watching". Made to run under a monitor.
• room_doc_watch.py (pure): new_lines(before, after) is counted, not set-differenced — a line written a second time is new again; addressed_to(lines, handles) matches the @ as a whole token, so @marshall and a bare mars in prose do not fire, and . : - bind only when a word character follows (@mars. at a sentence end still addresses mars; @mars:server does not match mars).

Evidence

Live, production, two processes on my own room's document. Watcher:

watching !KFZ…:ag2.space for ['mars', '@sutando-qingyun-001:ag2.space'] (94 chars now)

A second process appended three lines — one for @mars, one for @echo that mentions "mars" bare in prose, one for @marshall. The watcher printed, within a second:

MENTION	watch-probe-7731: @mars please look at this line

and nothing for the other two. (The first cut also fired on the bare "for mars" line; the @ is now required — the summon always writes it. The document was restored after the probe and checked against the original text.)

Tests (tests/room-doc-watch.test.py, rc=0): the pure rules (counted new lines, blank lines never new, whole-token @ matching, CJK and spaces in a handle, no handles → no lines); and the live half against a real Doc with a second peer's update applied — a remote edit is yielded, the agent's own write is not, the session ending raises, and the full snapshot→diff→address path finds exactly the mentioning line.

lint-skill PASS; review-checks PASS; neighbouring suites room-doc-cli, room-doc-lifecycle, room-doc-authors, room-doc-board-client rc=0.

Scope

Additive: one iterator on the client, one subcommand, one pure module, one SKILL.md section ("Staying in the document"). Nothing existing changes. This is the primitive for treating an in-document mention as an interrupt rather than a queued task — the policy of what to interrupt is the owner's call and not in this PR.

🤖 Generated with Claude Code

[qingyun-wu]

Pushed 8b4d9c449 with two fixes found by measuring the first version against production, both pinned:

1. Closing the iterator ended the session. asyncio.ensure_future(self._ended) returns the session's own future unchanged, so cancelling the waiter cancelled the session and the next write raised. Now shielded. Control: unshielding fails test_closing_the_iterator_leaves_the_session_alive.

2. One push per keystroke, no batching. Measured: 7 transactions 150 ms apart → 7 pushes, each arriving within the gap. So a person typing @mars please look was 18 pushes and the watcher printed the growing line on each. changes(settle=…) now yields once after the edits go quiet; watch uses --settle (default 1 s). Measured again live: 18 keystrokes → one MENTION line. Raw mode (settle 0) still yields every push, pinned separately.

[qingyun-wu]

Pushed 140c61cd4: watch generalised from one event on one kind to every event an agent in a document should hear about that is observable today, per the owner's list ("list the event-driven triggers and start building them"):

kind	where	when
mention	text	a new line @-mentions a handle
mention	board	a text label newly names a handle (a label that merely moved is not news)
assigned / moved / unassigned	kanban	a card's assignee becomes the handle / a card that is the handle's changes column / it is tombstoned
peer_joined / peer_left	any	by identity, so one person on two devices is one peer

RoomDoc.events(handles) is the single stream; watch prints EVENT<TAB>kind<TAB>where=… <TAB>text per item. Not included, because it needs the server's authorship record (#1256): someone edited the paragraph you wrote.

Live, production, one writer and three watchers (markdown / board / kanban on the same room): the text mention, the board label and the card assignment each produced exactly one EVENT line on its own watcher, and the cleanup produced the unassigned. Every probe was removed afterwards and the document text checked against its original.

Tests: 4 pure cases for the new rules (with a mutation control — disabling the board and kanban rules fails 5 cases, including the live ones), 4 live cases through events() with a second peer's update applied, and the earlier settle/close cases. review-checks PASS on the cumulative branch diff (the pre-push hook caught an import block the per-commit gate had not).

[qingyun-wu]

BLOCKING at 140c61cd473.

skills/room-doc/scripts/room_doc.py:143-147 writes event fields and document text directly into a tab-delimited, newline-delimited protocol without escaping. Board labels and kanban card text are arbitrary multi-line strings, so one legitimate event can become multiple output records; worse, document content can synthesize a second line beginning with EVENT that a monitor cannot distinguish from a real event.

I reproduced this against the current watch() implementation with one assigned-card event whose text was first line\nEVENT\tpeer_left\twho=@victim:x. The command printed two apparent records:

EVENT  assigned   where=kanban card=c1   first line
EVENT  peer_left  who=@victim:x

That violates the advertised “one line per event” contract and lets an untrusted document forge downstream triggers. Tabs in text or metadata also shift fields. Please serialize each event as a single unambiguous record, preferably compact JSON per line (or rigorously escape all control characters), and add round-trip tests with newline, tab, and an embedded EVENT prefix in board/kanban text.

The snapshot/diff rules themselves are sensibly separated and the settle batching addresses the measured keystroke burst. I could not run the pycrdt-backed suite in this isolated checkout because pycrdt is unavailable; the serialization reproduction is pure and executed directly against this head.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

BLOCKER STILL PRESENT at b3f29334044.

I re-fetched the new head. The reconnect/presence work does not touch the output encoding: watch() still interpolates detail and every metadata value directly into tab-separated output, so the exact multiline-text reproduction from my prior review still produces a forged second EVENT record. The function signature changed, but the vulnerable print expression is byte-for-byte the same.

The new head also adds no control-character or record-round-trip test. Please do not treat the reconnect work or pending CI as resolving the protocol-integrity finding; it remains blocking until each event has an unambiguous single-record serialization.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 89%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• skills/room-doc/scripts/room_doc.py (97.1%): Missing lines 256
• skills/room-doc/scripts/room_doc_client.py (100%)
• skills/room-doc/scripts/room_doc_protocol.py (100%)
• skills/room-doc/scripts/room_doc_watch.py (100%)

Summary

• Total: 206 lines
• Missing: 1 line
• Coverage: 99%

skills/room-doc/scripts/room_doc.py

Lines 252-260

  252         return await doctor(args)
  253 
  254     token, url = resolve_token(args.token), resolve_url(args.url)
  255     if args.command == "watch":
! 256         return await watch(args, token, url)
  257 
  258     async with open_room_doc(url, args.room, token, kind=args.kind,
  259                              insecure=args.insecure) as doc:
  260         if args.name:

---

[yixuan-ag2]

Reviewed at b3f293340448. Not approving yet — checks are still in flight (state quoted at the end). One finding, and it is measured rather than read: room_doc_watch.py imports nothing, exactly as its docstring promises, so I ran it.

What I verified by executing it

new_lines being counted, not set-differenced is the right call and it holds on every property I could think to break it with:

dup written twice      "a"      -> "a\na"       ['a']          new again  ✓
deletion is not news   "a\nb\nc"-> "a\nc"       []                        ✓
reorder is not news    "a\nb"   -> "b\na"       []                        ✓
edit in place IS news  "a"      -> "a-edited"   ['a-edited']              ✓

A set difference would have swallowed the first case silently, which is the one that matters for a summons written twice.

The handle pattern is tighter than its comment claims:

'@mars please look'    True     '@marshall hi'   False
'ask @mars.'           True     '@mars.b'        False
'bare mars here'       False    'bob@mars'       False   <- email, and the lookbehind catches it

The (?<!\w) guard means an address like bob@mars cannot fire the watcher. Worth keeping — nothing in the tests or the comment names that case, so it is currently an accident that a future edit could drop.

The finding: a card that disappears is silent, and the same file gets this right for peers

kanban_changes iterates after.items() only. So a card present in before and absent from after is never visited, and no unassigned is emitted:

before = {"c1": {"assignee": "@mars", "column": "doing"}}

after = {"c1": {..., "deleted": True}}   ->  [{'kind': 'unassigned', 'card': 'c1'}]   ✓
after = {}                               ->  []                                       ✗ silent

The docstring covers the tombstone — "a card deleted (tombstoned) is reported as unassigned" — and that path works. What is uncovered is the key simply going away.

The reason I think this is an oversight rather than a decision is peer_changes, twenty lines below, which handles precisely this shape:

peer_changes([{"id": "u1", "name": "mars"}], [])  ->  [{'kind': 'peer_left', 'who': 'u1'}]

It computes now - was and was - now. kanban_changes computes only the forward direction. Same file, same author, same question, two different answers — and the test suite has test_peers_arriving_and_leaving_by_identity_not_device for the peer case and no equivalent for cards.

The question that settles it: is the cards map guaranteed to tombstone rather than remove the key? If deletion always sets deleted: True and the key survives, this is unreachable and I will drop it — say so and I am satisfied. If any path removes the key, a watcher's "my cards" list goes stale with no event, which is the failure this module exists to prevent. The fix is a second loop over before for ids missing from after, mirroring peer_changes.

State, quoted rather than summarised

18 distinct check(s) from 18 entries (0 superseded)
  failing (check-runs): none
  non-success contexts: none
  running: ['typecheck + node & shell tests', 'diff coverage >= 95% (python)']
  ⚠ 1 REQUIRED context(s) ABSENT from the rollup: tsc + tests (clean install)

Two checks in flight and one required context absent, so this is not green and I will not put an approval in front of a batch that has not finished. gating-approvals reads 0 of 2. Ping me when it settles and I will come back — the finding above is the only thing standing between this and my approval, and it may well be a non-issue.

[github-actions]

@cla-assistant check