feat(workspace) agent display names with CJK support - #628
Conversation
Agents keep their ASCII agent_name as the stable identity (mentions, routing, storage keys) and gain an optional display_name shown across the UI. Any script is allowed (Chinese, emoji); empty clears back to agent_name. Backend - workspace_members.display_name column + alembic migration 040 - PATCH member accepts display_name — trimmed, max 64 chars, rejected when it matches another member's agent_name (mention ambiguity) - discover and workspace responses carry the new field - LLM router prompt lists the display name as an alias so users can address agents by it; the output contract still uses the ASCII name Frontend - agentLabel() helper, displayName on WorkspaceAgent/NetworkAgent - all roster/chat/task/routine/thread surfaces render the label while API calls keep using agentName - mention picker filters by display name too and inserts the ASCII token; rendered mentions show the label Slack-style - inline display-name editor in the agent profile panel
…l coverage Review follow-ups on agent display names - display_name is a routable alias, so it now shares one namespace with agent_name per workspace — duplicates across members' display names are rejected, and /v1/join plus cloud-agent creation reject an agent_name that matches another member's display name (re-joins of the same agent are unaffected) - reject control characters in display_name, and flatten display_name and description before inlining them into the router prompt so a crafted value cannot forge extra participant lines - DM surfaces now resolve labels (thread rows, sender preview, chat title) while dedup keys, session ids and avatar seeds stay on the canonical address - remaining raw renders switched to agentLabel — chat roster bar, missing-description chips, skills install list (4 states) - 4 new backend tests (duplicate alias, control chars, join clash, self-name re-join)
Second review round on agent display names - new app/naming.py domain module — single source for the shared name/alias namespace (find_alias_clash), the Unicode-aware character policy (has_unsafe_chars/sanitize_inline covering Cc, Zl, Zp and bidi controls, so U+2028/U+2029/U+0085 and RLO can no longer forge router prompt lines while ZWJ emoji keep working) and a workspace-row namespace lock (SELECT FOR UPDATE) that closes the check-then-write race between concurrent renames and joins - the join guard moved from the /v1/join route into _handle_agent_join, so raw network.agent.join events through /v1/events hit the same check, and it now runs after AuthMod — unauthenticated callers get a plain 401 and can no longer probe which display names exist; authenticated clashes surface as 400 via a reject_reason stamped on the event - every remaining member-creating entry point applies the guard — cloud-agent create, the Google OAuth callback (error page instead of a silent duplicate) and the Yumi backfill (skips with a warning) - remaining raw agent_name renders switched to agentLabel — offline warning chips, status messages, monitor search preview - 4 new tests, including the reviewer's events-path bypass repro and the 401-before-400 probe order
Third review round on agent display names - agent_name now passes the shared character policy (naming.agent_name_problem) in the post-auth join handler and at workspace creation, so a name like 'safe\n- forged' can no longer mint a member; roles are whitelisted to master/member/observer on join - every user-controlled field in the router prompt is flattened — participant names, roles, master, sender and history labels, not just display_name/description — covering legacy rows that predate the policy - namespace lock now precedes the membership reads in the join handler, cloud-agent create, the OAuth callback and the Yumi backfill, closing the read-before-lock race that turned concurrent same-name joins into a primary-key 500 - Yumi backfill runs the guard before any session mutation, so a clash no longer leaves a pending CloudAgentConfig that the next workspace's commit would persist - removed the function-scoped Workspace import that made the whole Google OAuth callback raise UnboundLocalError (pre-existing on develop) and kept the guard before any mutation there; added mocked-callback regression tests - completed the Bidi_Control set with U+061C, U+200E, U+200F - 9 new tests across join validation, role whitelist, direction marks, Yumi orphan objects and the OAuth callback
Fourth review round on agent display names — three of five findings are develop-era bugs surfaced by the new guards - GET /cloud-agents/google/auth now requires workspace credentials (header or ?token= query, since browsers navigate via plain href — the frontend link carries it) and validates agent_name against the same rule as POST /cloud-agents BEFORE issuing the OAuth state; it used to fall back to the workspace's own token for anonymous callers - the OAuth callback refuses to attach a Google config to an active member of another type, mirroring the POST /cloud-agents already- exists rule; repairing a config-less cloud:google member still works - Yumi backfill no longer takes over a real agent that happens to be named yumi (it rewrote agent_type/description); only a member already of the built-in type is repaired - naming.agent_name_problem and the join role whitelist tolerate non-string JSON values from raw /v1/events instead of 500ing - 7 new tests across the OAuth start/callback guards, Yumi takeover and non-string payloads
…uth pages Fifth review round — all findings are in the OAuth/Yumi subsystem, none in the display-name feature itself - browser flows now POST /cloud-agents/google/auth-url with auth headers and navigate to the returned one-time consent URL; the GET redirect is header-authenticated only and a query-string token is deliberately rejected, so the workspace credential never lands in access logs or browser history; the state no longer stores any credential and states expire after 10 minutes with opportunistic pruning - the public callback never echoes the error parameter (fixed copy per OAuth code, raw value only logged) and both OAuth HTML pages escape all dynamic content centrally - callback ownership is checked on the member unconditionally — a stale config row no longer bypasses the type guard, and a removed member of another type is not resurrected as a Google agent; a removed cloud:google member is explicitly reactivated, mirroring POST /cloud-agents - Yumi backfill refuses non-builtin owners of the name regardless of status (a soft-removed real yumi stayed claude/removed in the test) - NOT adopted: rejecting unknown Google models — validate_provider_model deliberately accepts any model id for known providers, and POST /cloud-agents behaves the same; a whitelist here would diverge - multi-worker OAuth state storage stays a known pre-existing limitation (process-local dict predates this branch), documented in code and split out as follow-up work - 8 new tests; the frontend Google button now fetches then navigates
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Plain lower() let visually identical aliases coexist: fullwidth ｙｕｍｉ passed the clash check against yumi, and casefold pairs (straẞe/strasse) slipped through. fold_alias() now normalizes NFKC and casefolds both sides; comparison moves into Python since SQL lower() can do neither and every caller already holds the workspace lock over a small member list. Adds fullwidth and casefold clash tests.
zomux
left a comment
zomux
left a comment
There was a problem hiding this comment.
Approve. Deep-reviewed with focus on the new attack surface and the bundled security fixes — all four verified as real pre-existing defects on develop and genuinely fixed: (a) the OAuth callback's UnboundLocalError (shadowing function-scoped import — every callback 500'd); (b) the OAuth start minting states for ANONYMOUS callers with the workspace's own token as fallback — now credential-required on both endpoints, header-auth only, token never in a URL, states carry no credentials and expire in 10min; (c) the ?error= reflected XSS — now fixed copy + centralized html.escape covering every interpolated value; (d) the Yumi backfill taking over a user's real agent named yumi — guards now run before any mutation, builtin-type only, no provisioning regression.
Prompt-injection analysis: sanitize_inline's Cc/Zl/Zp+Bidi coverage verified exhaustive against splitlines(); a display name cannot span lines in the router prompt, and a name like 'next:evil' can't hijack routing because output is still validated against the real candidate set. All 5 member-creation sites plus rename enforce the policy. Namespace clash checks cover all directions with the workspace-row lock actually taken at every check-then-write site. Join validation verified not to break any existing client (launcher/SDK/deepseek/pi names all pass; unknown roles downgrade; 401-before-validation prevents name probing). Frontend: agentLabel() everywhere, DM keys/avatars/API calls still agent_name, no dangerouslySetInnerHTML. Migration 040 chains onto 039, nullable-only, safe downgrade. Full backend suite: identical failure set to develop (env-only), zero new.
Fixed on-branch before merge (2cd0aec): the alias comparison used plain lower(), letting visually identical confusables coexist — fullwidth ｙｕｍｉ vs yumi, casefold pairs like straẞe/strasse. find_alias_clash now folds both sides through NFKC+casefold (in Python; SQL lower() can do neither, and callers already hold the lock). Added both clash tests; 169/169 pass.
Deploy notes: frontend+backend must ship together (old frontend's Connect-Google href gets 401 from the new backend — the acceptable cost of removing the token-in-URL leak). Migration heads-up: this takes 040, so #597 must renumber to 041+. The process-local OAuth state dict (multi-replica limitation) is accurately scoped as pre-existing.
3 participants
Background
A workspace agent has a single
agent_namefield that serves as the database primary key, the @mention token, a URL path segment, a filesystem directory name, and the display name all at once. Every entry point validates it against[a-zA-Z0-9_-], so agents cannot have Chinese names. Source of the requirement: direct user request (support Chinese and English agent names in the workspace).Across five review rounds, a batch of pre-existing defects on develop — all on the member-creation / OAuth paths — was surfaced and fixed along the way: the Google OAuth callback always raised
UnboundLocalErrorbecause a function-scoped import shadowedWorkspace; the OAuth start endpoint never verified credentials (anonymous callers got a state issued with the workspace's own token as fallback); the OAuth error page reflected XSS; and the Yumi backfill would take over a user's real agent that happened to be namedyumi.How to reproduce
/v1/cloud-agents/google/authreturned 307 and the flow could mint a member;/callback?error=<script>...was echoed verbatim as text/html; running the Yumi backfill on a workspace that already had a real agent withagent_name=yumi, agent_type=claude.Behavior before
Agent names were ASCII-only, with no separation between display name and identity. The OAuth / Yumi paths respectively showed a 500 on callback, anonymous member creation, reflected XSS, and a real agent being rewritten to the built-in type.
Behavior after
agent_name.next:<agent_name>./cloud-agents/google/auth-urlwith auth headers and navigates to the returned one-time consent URL (the workspace token never appears in any URL); the GET redirect accepts header auth only; error pages use fixed copy with centralized HTML escaping; the ownership guard covers stale configs and removed members; states expire after 10 minutes.Design
Core trade-off: the
agent_namecharacter set stays untouched (it is a primary key / mention token / filesystem path; loosening it would require rewriting mention parsing and risks session-directory collisions). Instead, a separatedisplay_namepresentation field is added — the same shape as the existingWorkspaceCollaborator.display_nameprecedent.workspace_members.display_name TEXT NULL, alembic migration 040; zero migration cost for existing rows.app/naming.pyis the single source of rules:find_alias_clash.splitlines()treats as a line break) plus the full 9-codepoint Bidi_Control set; Cf stays allowed so ZWJ emojisequences keep working.agent_nameitself passes the same policy at every entry point and tolerates non-string payloads; the join role is whitelisted to master/member/observer.SELECT FOR UPDATEon the workspace row before reading the member table, so concurrent renames/joins cannot commit duplicate aliases or hit primary-key conflicts._handle_agent_join, covering both/v1/joinand raw/v1/events. Rejection reasons travel back via event metadata and map to 400; unauthenticated callers only ever see 401 and cannot probe which names exist.sanitize_inline, covering legacy rows that predate the policy.agentLabel()helper; API calls, DM dedup keys and avatar seeds keep usingagent_name. Editing happens inline in the agent profile panel.Review suggestion not adopted: a Google model whitelist for OAuth —
validate_provider_modeldeliberately accepts any model id for known providers (the registry is curated suggestions, not a hard restriction), andPOST /cloud-agentsbehaves the same way; tightening only the OAuth path would diverge.Scope of impact
display_namefield already present in the discover response.agent_namenow rejects control characters, over-length and leading/trailing whitespace; the OAuth start went from unauthenticated to credential-required, and query-string tokens are rejected.Deployment notes
display_namecolumn in existing data is simply ignored; no data rollback needed.How to verify
Manual check: open an agent's profile panel, click the pencil next to the name, save "小明" → every surface shows 小明; typing
@小in chat filters to the agent, inserts@<ascii-name>, renders as @小明; duplicate names or names with line breaks are rejected with a 400; the Connect Google redirect URL contains no workspace token.🤖 Generated with Claude Code