Repository navigation
feat(social): sync Bot avatars and decline grants on remove (#137, #139) - #147
Conversation
Owner-chosen avatar/color is sent with update_bot_profile whenever a local Bot's look is known, and remote looks render from optional server fields. Remove this Bot calls decline_bot_grant so it leaves the owner's Manage access; older servers still hide locally. Co-authored-by: Danyang Chen <birfychen@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a73b338af5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof member.color === 'string' && member.color && bot.color !== member.color) { | ||
| bot.color = member.color | ||
| changed = true | ||
| } | ||
| if (member.avatar && typeof member.avatar === 'object') { |
There was a problem hiding this comment.
When a peer resets its avatar or color, list_remote_bots can supply a new avatarUpdatedAt while omitting the cleared field. These branches only copy fields that are present, after which the function records the new timestamp; every later refresh then takes the timestamp fast path and the persisted remote chat keeps rendering the old avatar/color forever instead of the handle-seeded fallback. For a versioned payload, clear missing look fields before committing the new version.
Useful? React with 👍 / 👎.
| if (settings.lookSynced?.[bot.id] === fingerprint) return | ||
| const empty = !('avatar' in patch) && !('color' in patch) | ||
| if (empty && !settings.lookSynced?.[bot.id]) return | ||
| const body = empty ? { avatar: null, color: null } : patch |
There was a problem hiding this comment.
When a Bot retains one look component while clearing the other (for example, a colored Bot removes an image-only avatar), empty is false and this body omits the cleared field. update_bot_profile/the PATCH therefore leaves that old server field unchanged, but the subsequently saved fingerprint contains null for it, so the stale value is considered synced and is never retried. Build the request with an explicit null for each independently absent avatar/color field.
Useful? React with 👍 / 👎.
| const member = memberOf(bot?.id) | ||
| if (!member?.bot_id || typeof api.updateBotProfile !== 'function') return |
There was a problem hiding this comment.
In the supported legacy token + botId mode with no home, local Bots never receive a social.member field because home-member registration is not used. This guard therefore returns on every startup/poll/roster sync, so updateBotProfile is never called and the configured legacy Bot's avatar/color cannot be published. In no-home mode, use config.botId for the configured localBot/Main Bot as the profile identity.
Useful? React with 👍 / 👎.
| return code === 'unknown' || /unknown tool|no tool|not found|method not found|unrecognized/i.test(text) | ||
| const blob = `${code} ${text}` | ||
| if (/grant_not_found/i.test(blob)) return false | ||
| return code === 'unknown' || /unknown tool|no tool|method not found|unrecognized tool/i.test(text) |
There was a problem hiding this comment.
On an older MCP server that reports an absent tool as Tool update_bot_profile not found (without code unknown), this predicate now returns false even though the previous predicate explicitly matched not found. The profile/decline calls then throw before their REST or local-hide compatibility fallback, and the existing list/revoke grant fallbacks regress for the same response. Keep recognizing tool-specific not found messages while continuing to exclude grant_not_found.
Useful? React with 👍 / 👎.
Client side of Bot Social PR #6 (now deployed): fixes #137 and the other half of #139.
update_bot_profile/PATCH /v1/bots/{id}/profile) when changed and on startup for unsynced Bots; remote Bots'avatar,avatar_kind,color,avatar_updated_atare rendered in lists, rooms, tiles and pickers (cached byavatar_updated_at; falls back to the id-based color).decline_bot_grant(after the single confirm) so the owner's Manage access no longer lists it;regranted,grant_not_foundand an older server (404) are handled.Verified:
ARCH_DOCS_BASE=upstream/main npm run verifypasses.