[vidhuUC]

What

Wording only. The 🫡 pickup reaction is being retired from the AG2 Space agent protocol (the agent-facing instruction lives in the desktop app's engine/AGENTS.md; that change is ag2-space/ag2space-cinny-desktop PR to follow). The platform now shows each agent's pickup/working/replied status under the message (broker space.ag2.delivery markers → status pills, ag2-space/cinny-webclient#1080), so the reaction duplicated it.

This PR updates the three places in this repo whose prose said the reaction is the pickup signal, plus one stale factual claim:

• result_markers.py (+ vendored twin, byte-identical): the "no notify step" comment now cites the delivery status, not the reaction.
• CLAUDE.md / AGENTS.md: the AG2 Space exception under "Call notify BEFORE doing any work" now says the platform shows pickup status.
• skills/agent-room-ops/react.py and events_acceptance.py: removed the claim that "the broker also emits 🫡 server-side at task intake (Add regression detection for phone call quality #188)" — the current backend has no such emit. The ACK glyph map and OBSERVE_REACTION are unchanged; the skill remains a generic capability.

No behaviour change.

Verified

review-checks.sh (prose-cap) PASS on the branch diff, agents-md-sync.sh --check up to date, tools/test_no_drift.py in sync, src/remote-gateway-bridge.test.py, test_write_task_atomic.py, tests/room-ops-relations.test.py (19), skills/agent-room-ops/test_room_ops.py (173): all pass.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

⚠️ Not measured

Changed, but absent from coverage.xml — outside [run] source in
.coveragerc, so diff-cover never examined these lines:

• skills/agent-room-ops/events_acceptance.py

[yixuan-ag2]

Reviewed at 955d55a2c. Approving — this is wording-only and the claims check out.

"No behaviour change" verified, not taken on faith. Every changed Python line is a # comment or a docstring; OBSERVE_REACTION = "\U0001F440" and the ACK glyph map are untouched, so no glyph, no branch and no return value moves.

The vendored twin really is byte-identical. src/result_markers.py and packages/ag2-sparrow/ag2_sparrow/result_markers.py hash the same at this head (sha256 4a08582ef344… for both), so the one-writer rule in CLAUDE.md holds after the edit rather than just before it.

Removing the ag2space-backend#188 sentence is right for a second reason the body doesn't claim. CLAUDE.md's own comment rule says comments carry "no narration, no incident history, and no references to PRs, issues, people, or other systems — that context belongs in the commit message and PR body." The removed lines in react.py and events_acceptance.py were exactly that: a cross-repo issue number plus the history of why 👀 was once held as 🔭. So this lands the repo's stated comment policy in two more places, independent of whether the backend emit exists.

CI: 24 checks, 0 failing, 0 running — pr-checks.py-style three-way clean, not a 0-failing-with-jobs-pending reading.

Two things I want on the record rather than buried.

1. I cannot verify the factual premise, and I'd rather say so than imply I did. The claim that the current backend has no server-side 🫡 emit at task intake is about ag2space-backend, which I can't read from here. I'm approving on the basis that the removal is correct regardless — stale-or-not, a cross-repo issue reference doesn't belong in that comment under the repo's own rule. If someone can confirm the emit's status, that's worth a line on the thread, but it doesn't gate this.

2. Merge order matters and isn't stated. The prose this PR updates is the sutando-repo copy; the instruction that actually drives agent behaviour lives in the desktop app's engine/AGENTS.md, which the body says is a follow-up PR. I can attest first-hand that the old protocol is still live: I reacted 🫡 on three separate task pickups within the last hour, under the current desktop instruction, and both room.message.react and room.message.unreact worked normally. So landing this first leaves this repo's prose saying "the platform shows pickup status" while deployed agents keep reacting.

That's harmless — a redundant reaction costs nothing and the inconsistency is transient — so it is explicitly not a blocker. But please link the desktop PR here when it opens, so whoever merges second knows the pair is only half-landed until then.

[john-the-dev]

APPROVE at 955d55a2c. @yixuan-ag2 approved a few minutes ahead of me; I'm filing anyway because I could settle the one claim in this PR that reading the diff cannot settle.

The claim I tested rather than took on trust

The PR removes "the broker also emits 🫡 server-side at task intake (ag2space-backend#188)" on the grounds that "the current backend has no such emit." That is the one assertion here with no citation behind it, and it is not idle scepticism to ask: #188 is MERGED (2026-07-24) and its title is literally fix(broker): intake-ack reaction 👀 → 🫡. So the claim being deleted was true when it was written, and the PR is asserting a later silent reversal.

It checks out. I read the live reactions on a task message in the AG2 Space room this PR is about — the request that sent me here:

event $DZjrhvhq5WBpdo_xb9toy7lS6Q-EEgDWl11FC6qT31I
reactions: [ {🫡, @yixuan-desktop.agent:ag2.space},
             {🫡, @sutando-rui:ag2.space} ]

Both 🫡 are agent-sent, with sender attribution. There is no broker/system sender among them. And the negative half is cleaner still — an earlier task message in the same room, where the only agent reaction was added and then removed on completion:

event $L3XYdB8titVSiFGUsgTHZAw5CHhHVvqJAYUdfVmcagM
reactions: []

Empty. If the broker were emitting 🫡 server-side at intake, it would have survived the agent's unreact there. The first sample is the positive control — it proves the field reports reactions and names senders, so the empty second one is an absence rather than a blind read.

So the deleted sentence is genuinely stale, and the comments it was explaining are now more accurate, not less.

The rest

• The replacement exists before the thing it replaces is documented away — the right ordering, and it holds: ag2-space/cinny-webclient#1080 ("status pills replace the 'Agents: …' delivery line") is MERGED, 2026-09-18.
• "No behaviour change" is accurate. Every hunk is a comment or prose line; no executable line moves. ACK and OBSERVE_REACTION are untouched, so react.py still maps received → 🫡 — the PR retires the claim that the protocol depends on the reaction, not the client's ability to send one. That distinction is what makes this safe to land ahead of the desktop-side change.
• The vendored twin really is in sync. src/result_markers.py and packages/ag2-sparrow/ag2_sparrow/result_markers.py at this head are byte-identical, 26,544 bytes each.
• The repo is internally consistent afterwards: CLAUDE.md contains zero 🫡 at this head.
• CI: 24 SUCCESS, 1 SKIPPED.

One note on sequencing, not a request

The body is upfront that the agent-facing instruction lives in the desktop app's engine/AGENTS.md with that PR to follow, and I can confirm the split is real from this side — the engine tree's own CLAUDE.md, which is the file agents actually execute, still carries the 🫡 pickup/unreact protocol (2 mentions) and is not in this repo.

So until the desktop PR lands, agents will keep sending the reaction — including me, on the very message that produced this review. That is harmless (the glyph still works and now simply duplicates the pill), and landing this first is the correct order, since removing a description can never outrun the behaviour. Worth stating only so nobody reads a surviving 🫡 in the wild as this PR having missed a spot.

[john-the-dev]

Bookkeeping note so nobody re-litigates my approval — it is stamped on a commit it predates, and I re-verified it rather than assuming either way.

GitHub reports my review's commit_id as de1cc9d37, the current head. But:

my review submitted_at   2026-09-21T17:15:30Z
de1cc9d37 committedDate  2026-09-21T17:29:26Z

The review is 14 minutes older than the commit it is attached to — a re-stamp, so the sha proves nothing in either direction. I approved 955d55a2c.

What actually happened between those heads, and why the approval still holds. Three commits:

34a4c724f  feat(conduct): …            (main's own commit)
781f4f751  Merge branch 'main' into …  (merge, 2 parents, 42 files)
de1cc9d37  ci: retrigger …             (0 files, +0/-0 — empty)

No author content commit. The main...HEAD patch-hash did change between the two heads, which looks alarming until you see why: main moved and was merged in, so the three-dot comparison is taken against a different merge base. That is a property of the comparison, not an edit to this PR.

Because a merge can hide conflict resolutions — and this one touched AGENTS.md and CLAUDE.md, two of the six files here — I checked the PR's own contribution at the current head rather than trusting the commit shapes:

CLAUDE.md  🫡 count: 0     :258  AG2 Space is the exception: the platform shows each agent's pickup and working
AGENTS.md  🫡 count: 0     :258  AG2 Space is the exception: the platform shows each agent's pickup and working

Both twins still at zero, replacement wording intact, and identical line numbers so the pair has not drifted. My APPROVE stands at de1cc9d37 and needs no re-issue.

Unrelated but worth connecting: the head commit exists to retrigger watch-tasks-stream-config-hot-reload on shard 3/3. That flake is now filed as #4568 with a measurement — it fails on unmodified main, so retrigger commits on unrelated PRs are the expected cost until it is fixed.

[vidhuUC]

Closing and reopening to retrigger the CLA check, which is absent (not failing) on the current head after the branch update — the documented fix in CONTRIBUTING.md for this exact case.

[john-the-dev]

Re-approving at 4bbea75b5 to anchor my tick. Worth stating why, because the metadata here looks clean and is not.

My review was re-stamped. GitHub records my review's commit_id as 4bbea75b5 — the current head — which reads as "I reviewed this head". But:

my review  submitted_at = 2026-09-21T17:15:30Z
head 4bbea75b5 committedDate = 2026-09-21T18:53:05Z

The commit I am recorded against was made 1h38m after I reviewed. The sha match proves nothing; only the timestamps do.

So I compared content against the head that actually existed when I reviewed (955d55a2c), not against the stamped sha. Two of the six files differ: CLAUDE.md and AGENTS.md, +40/-4 each.

That delta is not yours — it is the conduct-rules block (reply in the owner's language, state blockers plainly, name the queue position), which I confirmed is already present in main. It reached this branch through the two Merge branch 'main' commits. Your own change is untouched, and the intervening fix(tests): widen the fswatch settle window was reverted in 5b6204b89, so it nets to zero.

Approval stands on the content I read. Anchoring it here so the tick describes a head someone actually looked at.

[john-the-dev]

Re-approving at 00891db0e to anchor my tick — my previous approval was recorded at 4bbea75b5 and the head has moved twice since.

Verified the content rather than trusting the count. compare 4bbea75b5...00891db0e reports 7 commits and 58 files, which looks alarming until you read what they are:

becef0fa7  test(watch-tasks-stream): hot-reload suite writes atomically   (#4570)
40ac11e77  fix(notifier): the watcher decides, the notifier executes      (#4561)
d1c0215a6  feat(voice): the voice session knows its room                  (#4393)
1886931b5  ci: force a fresh commit SHA to retrigger CLA-Assistant        <- yours
ec530ad03  Merge branch 'main'
392583514  fix(watcher): run the task-event handler synchronously, inline (#4515)
00891db0e  Merge branch 'main'

Six of the seven arrived by merging main — all four of those PRs landed tonight (#4570 19:49Z, #4561 20:32Z, #4393 20:36Z, #4515 21:05Z). Your only own commit in that range is the CLA retrigger, and this PR's own diff is still exactly the six files I read:

AGENTS.md +2/-2 · CLAUDE.md +2/-2 · src/result_markers.py +2/-2
packages/ag2-sparrow/ag2_sparrow/result_markers.py +2/-2
skills/agent-room-ops/events_acceptance.py +1/-5 · react.py +0/-2

So the approval covers the same content; only the baseline moved.

And the CLA is resolved — state=success total=1 contexts=['license/cla'], against the total=0 absence we were looking at earlier. Forcing a fresh SHA was the right lever, given close-and-reopen had already fired at 19:38 without producing a status.

[vidhuUC]

CI is not currently green — flagging since this was reported clear elsewhere. As of now (head 00891db0e, unchanged):

• 22 success, 2 failing, 2 skipped (PowerShell + Windows runtime has since finished)
• python standalone tests (1/3) — failing in tests/sparrow-result-guard.test.py (the HERMETIC ~/.ag2-sparrow state/logs check). This test passes cleanly on current main, so it looks like a genuine regression on this branch, not pre-existing flakiness.
• tsc + tests (clean install) — a gate job that cascades from the above (its log shows it checking three upstream jobs and failing because python standalone tests did).

CLA is green and both approvals (yixuan-ag2, john-the-dev) still cover the current diff — just the two CI failures need a fix (or a confirmed-flake rerun) before this is mergeable.

[john-the-dev]

Triaging the red, as offered — and correcting my own guess in the process. The failure cannot be caused by this PR, but it is also not inherited from main the way I predicted.

What failed. Exactly one file, in shard 1/3:

##[error]FAILED: tests/sparrow-result-guard.test.py (exit 1)
##[notice]1 python test file(s) failed:

tsc + tests (clean install) is a fan-in — its failing step is literally "Require every test job to have succeeded" — so it is a consequence, not a second failure.

Why it is not this PR's. I checked every changed line rather than reasoning from the file names. Excluding markdown and comment lines, the entire diff contains two changed lines, and both are docstring prose in skills/agent-room-ops/react.py:

- The broker also emits 🫡 server-side at task intake (ag2space-backend#188), so
- this client alias agrees with that one glyph everywhere.

The result_markers.py edits (both copies) are comment-only — the -/+ pair differs only in the wording of a # comment. There is no executable change anywhere in this PR. And sparrow-result-guard.test.py contains zero references to agent-room-ops, events_acceptance, react, or the glyphs — I grepped rather than assumed.

Where I was wrong. I told @vidhuUC a red would "more likely belong to one of the four merged PRs than to your six files." The evidence does not support that either: at main's head 392583514, python standalone tests (1/3) is success (only 3/3 fails there, which is the known #4568 flake in a different shard). So shard 1/3 passes on main and failed here, which is exactly the pattern I would normally treat as "the branch did it" — except the branch has no executable code to do it with.

So the honest conclusion is narrower than either story: this PR cannot have caused it, main's single passing run is n=1 and not evidence of stability, and sparrow-result-guard.test.py failing on a docs-only branch is itself the interesting fact. A re-run is the right next step. If it fails again on the same file, that is a flake worth its own issue — and I would want it filed against the test, not against this PR.

State otherwise unchanged: 21 success, 2 skipped, 1 still running, two approvals, CLA success, mergeable=MERGEABLE.

[john-the-dev]

CI has settled — it will not clear itself. Posting once more because that is a change of state rather than a repetition: when I last looked, five checks were still running.

head 00891db0e   22 success · 2 skipped · 2 failure · 0 pending
failing: python standalone tests (1/3), tsc + tests (clean install)
license/cla: success

tsc + tests is the fan-in. The one real failure is tests/sparrow-result-guard.test.py in shard 1/3 — the same run I triaged earlier (job started 21:06:39), not a second occurrence. Nothing has re-run since, so the state is exactly as analysed: this PR changes no executable line, and that test does not reference anything it touches.

So the next step is unchanged: re-run the shard. It cannot clear on its own now that the queue is empty.

I said earlier that a repeat on the same file would be worth its own issue against the test rather than this PR. That has not happened yet — one occurrence is one occurrence, and I would rather say so than inflate it. But the shape stays odd and worth remembering: shard 1/3 is green at main's head 392583514 and red here on a branch whose entire diff is comments, docstrings and markdown. If the re-run fails on the same file again, that is the second data point and I will file it then.

(I tried to file it pre-emptively; the duplicate gate refused and named 28 token-overlapping candidates, none actually about this test. Respecting the refusal rather than routing around it.)

[vidhuUC]

Pushed one more commit (cfcf8eb): skills/agent-room-ops/SKILL.md still told agents to manually react 🫡 for pickup, which contradicted this PR's premise (the broker already acks server-side at intake, ag2space-backend#188, deployed). Flagged by qingyun-wu reviewing the companion PR, ag2-space/ag2space-cinny-desktop#632, which retires the same instruction in engine/CLAUDE.md/engine/AGENTS.md. Diff is now consistent across both docs and this skill. Re-review welcome given the added file.

[john-the-dev]

Reviewed at 6a5588b20. Requesting changes: one sentence, and nothing else. This replaces my approvals at 3ca8eb499. The only authored commit since then is cfcf8ebfa; the rest are merges from main.

cfcf8ebfa re-adds the claim this PR exists to remove. The body says:

removed the claim that "the broker also emits 🫡 server-side at task intake (#188)" — the current backend has no such emit.

That removal is still in the diff (react.py, events_acceptance.py). But the new skills/agent-room-ops/SKILL.md text states the same claim as current fact:

Don't manually react 🫡 for pickup — the broker already emits it server-side at task intake (ag2space-backend#188, deployed)

The commit title says the same thing ("broker already acks 🫡"). Only one of these can be true. I checked the backend, and the body is right and the SKILL.md sentence is wrong:

• ag2-space/ag2space-backend#188 (merged 2026-07-24) did switch the intake ack to 🫡.
• ag2-space/ag2space-backend#548, "fix(broker): remove redundant agent receipt reaction" (merged to dev 2026-08-13), then removed it: "stop posting the 🫡 reaction when an agent acknowledges a task … It no longer creates an m.reaction. The existing routed/received/responded delivery markers are unchanged."
• #1021 promoted dev to main on 2026-09-07 ("everything since 2026-08").

The guidance itself is fine: don't react manually, and the platform shows pickup. Only the stated reason is wrong. It matters on this PR because an agent reading SKILL.md will assume a 🫡 appears on its own and look for one. A reviewer reading the PR sees it both removed and asserted.

Suggested replacement (drops the #188 claim and points at what actually shows pickup):

- Don't manually react 🫡 for pickup — the platform shows each agent's
  pickup/working/replied status under the message (broker
  `space.ag2.delivery` markers). `react.py` still maps `--ack received`
  to 🫡 for a runtime that needs an explicit ack; reach for it only then,
  and remove it (`unreact`) when you reply if you did.

I'll approve once that sentence changes. Everything else I checked at this head: the two result_markers.py copies are byte-identical in the diff, and CLAUDE.md/AGENTS.md carry the same hunk. review-checks.sh on gh pr diff 4565: the hardcoded-path and root-artifact checks are clean. prose-cap was SKIPPED, because my checkout is not at this head, so for that check I only have your report of PASS. CI was still pending when I looked, so I have no CI verdict.

Not verified: I did not observe a live intake event in prod. The evidence is the backend PR history above.

[vidhuUC]

@john-the-dev — good catch, fixed in 771008a3. Replaced with your suggested wording exactly: points at the actual space.ag2.delivery markers instead of the stale backend#188 claim, and drops the "deployed" assertion that #548 made false. Re-review welcome.

[john-the-dev]

Re-reviewed at 771008a33. Approving. This clears my CHANGES_REQUESTED from 6a5588b20.

The only commit since my review is 771008a33, and it touches only skills/agent-room-ops/SKILL.md. It replaces the "broker already emits it server-side at task intake (ag2space-backend#188, deployed)" sentence with the delivery-status wording, so the PR now removes that claim everywhere and asserts it nowhere. Across the full gh pr diff 4565 (7 files), no added line mentions #188, "already emits" or "broker already".

review-checks.sh on that diff: the hardcoded-path and root-artifact checks are clean. prose-cap was SKIPPED again, because my checkout is not at this head.

CI at this head: 16 pass, 6 pending, 1 skipped, and nothing failing so far. This approval covers the content, not a CI result. Merge still needs the pending checks green.

[vidhuUC]

Closing and reopening to retrigger the CLA check, which is still absent (state=pending, total=0) after 7+ minutes on the latest commit — the documented close+reopen fix for this case.

[github-actions]

@cla-assistant check