Skip to content

fix(room-collab): watch defaults its presence name to the mxid's localpart - #4596

Merged
qingyun-wu merged 2 commits into
mainfrom
fix/room-collab-presence-name-default
Sep 22, 2026
Merged

qingyun-wu merged 2 commits into
mainfrom
fix/room-collab-presence-name-default

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

The problem

watch called set_presence only when --name was given:

if args.name:
    await doc.set_presence(args.name, user_id=args.user_id)

Omit the flag and you get a connection that looks joined to the agent and absent to everyone else. A nameless peer is not rendered anywhere — the web client's peersOf skips an awareness state without a non-empty name, and the service's presence summary counts such a peer without naming it.

That is the same shape as the read-instead-of-watch bug this follows: the command does not do the thing it appears to do, and nothing tells you. An mxid already carries a usable name — and it is the one the summon hint fills in — so the fix is to default to its localpart rather than require a flag nobody can tell they forgot.

The change

The derivation moves to presence_name() at module scope, so it is testable without a socket instead of being an inline expression inside watch's connection loop:

announce = presence_name(args.name, args.user_id)
if announce:
    await doc.set_presence(announce, user_id=args.user_id)

An explicit --name still wins. Nothing usable yields None, and the caller then skips set_presence rather than publishing an empty name the readers would drop anyway.

Evidence

tests/room-collab-presence-name-default.test.py, 10 cases:

explicit name wins, and is trimmed
blank name falls through to the localpart
@sutando-qingyun-001:ag2.space -> sutando-qingyun-001
@weird:host:port               -> weird        (localpart only)
@:server, "", not-an-mxid, None -> None        (caller skips set_presence)

Auto-discovered by scripts/discover-python-tests.sh (confirmed: 1047 tests found, this one among them), and it SKIPs cleanly on a host without the room-collab deps rather than failing.

Two mutants killed, because an assertion that cannot fail is not evidence:

drop the localpart branch        -> "localpart: got None, want 'sutando-qingyun-001'"
accept any string as an mxid     -> "malformed id: got 'ot-an-mxid', want None"

The second is worth reading twice: without the @/: guard, not-an-mxid becomes ot-an-mxid, which is precisely the garbage that guard exists to reject.

review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean). lint-workspace-resolution --diff: clean.

Not verified

That the published name reaches a viewer end to end. The presence summary returns counts today, so a name is not externally observable until ag2-space/ag2space-backend#1284 ships who. I verified the derivation and the call site, not the pixel.

Related

Pairs with ag2-space/cinny-webclient#1190, whose summon hint passes --name explicitly. This is the floor for an agent that omits it.

🤖 Generated with Claude Code

qingyun-wu and others added 2 commits September 22, 2026 00:45
…lpart

`watch` called `set_presence` only when `--name` was given, so omitting it
produced a connection that looked joined to the agent and absent to everyone
else. A nameless peer is not rendered anywhere: the web client's `peersOf`
skips an awareness state without a non-empty name, and the service's presence
summary counts such a peer without naming it.

That is the same silent-no-op shape as the `read`-instead-of-`watch` bug this
follows — the command does not do the thing it appears to do, and nothing says
so. An mxid already carries a usable name, and it is the one the summon hint
fills in, so default to its localpart rather than requiring a flag nobody can
tell they forgot.

The derivation moves into `presence_name()` at module scope so it can be
tested without a socket, rather than staying an inline expression inside
`watch`'s connection loop.

Verified:
- tests/room-collab-presence-name-default.test.py — 10 cases: explicit name
  wins and is trimmed, blank falls through to the localpart, `@weird:host:port`
  takes only the localpart, and a malformed or empty id yields None so the
  caller skips `set_presence` rather than publishing an empty name the readers
  would drop anyway. Auto-discovered by scripts/discover-python-tests.sh, and
  it SKIPs cleanly where the room-collab deps are absent.
- Two mutants killed: dropping the localpart branch fails "localpart: got None";
  accepting any string as an mxid fails "malformed id: got 'ot-an-mxid'" —
  which is the exact garbage the `@`/`:` guard exists to reject.
- review-checks and lint-workspace-resolution clean on the diff.

Pairs with cinny-webclient #1190, whose summon hint passes `--name` explicitly;
this is the floor for an agent that omits it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…test

CI's old-collab-literals ratchet failed: the new test added one retired
spelling (0 -> 1), and the ratchet forbids growth.

The env var was not needed. It was defensive — room_doc.py makes its store
directory at import — but this test loads the CLI module, whose client import
sits inside functions, so nothing touches that path. Verified by running the
test with and without the room-collab deps: both pass.

One root cause, three red checks: the ratchet runs in 'python standalone
tests (2/3)' and in typecheck's 'Lint skill manifests' step, and 'tsc + tests
(clean install)' is the rollup that requires both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu

Copy link
Copy Markdown
Collaborator Author

Blocking findings on 814d1a76:

  • skills/room-collab/scripts/room_collab.py:470-475: the fallback only sees args.user_id. The CLI already treats AG2SPACE_USER_ID / AG2_MATRIX_USER_ID as the caller's identity in own_handles, but a normal watch relying on either environment variable still passes None here and remains nameless/invisible. Resolve the same effective identity once, derive the localpart from it, and pass that identity to set_presence; add the environment-only case to the regression.

  • tests/room-collab-presence-name-default.test.py:21: the new test adds the retired ROOM_DOC_DB spelling, so both the manifest-lint job and Python shard 2 fail. The variable is unused by the imported CLI path; remove the vestigial setup (and its now-unused imports) rather than growing the legacy baseline. The direct 10-case test passes locally, but the required old-literal guard correctly makes the current head red.

Changed files compile and git diff --check is clean. Formal request-changes is unavailable because this account authored the PR.

Reviewed by Qingyun's Personal Codex.

@qingyun-wu

qingyun-wu commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Current-head follow-up for 49968e6e: the retired-literal failure is fixed, and the focused presence-name test plus old-literal guard now pass locally.

The remaining blocker is still present in skills/room-collab/scripts/room_collab.py:470-475: watch derives and publishes presence only from args.user_id, while the CLI's established identity path also accepts AG2SPACE_USER_ID and AG2_MATRIX_USER_ID. A normal environment-configured invocation therefore still calls neither presence_name nor set_presence with its effective mxid and remains invisible. Please resolve the effective user id from the same identity sources used by own_handles, use it for both the fallback name and set_presence, and cover an environment-only invocation in the regression.

Formal request-changes is unavailable because this account authored the PR.

Reviewed by Qingyun's Personal Codex.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

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

Diff Coverage

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

  • skills/room-collab/scripts/room_collab.py (100%)

Summary

  • Total: 10 lines
  • Missing: 0 lines
  • Coverage: 100%

@qingyun-wu
qingyun-wu merged commit 6162d84 into main Sep 22, 2026
23 checks passed
@qingyun-wu
qingyun-wu deleted the fix/room-collab-presence-name-default branch September 22, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant