diff --git a/docs/client-api.md b/docs/client-api.md index cab68bda2..813b03541 100644 --- a/docs/client-api.md +++ b/docs/client-api.md @@ -708,7 +708,7 @@ When the synchronous reply is an error envelope, the request was rejected before |----------|---------|----------|-------| | `limit` | number | no | If set, must be `> 0`. Caps the number of members returned. | | `offset` | number | no | If set, must be `>= 0`. For pagination. | -| `enrich` | boolean | no | When `true`, populates the display fields (`engName`, `chineseName`, `isOwner`, `sectName`, `memberCount`) on each entry. Omitted-or-`false` returns the lean record only. | +| `enrich` | boolean | no | When `true`, populates the display fields (`engName`, `chineseName`, `name`, `isOwner`, `orgName`, `memberCount`) on each entry. Omitted-or-`false` returns the lean record only. | ```json { "limit": 50, "enrich": true } @@ -738,8 +738,9 @@ When the synchronous reply is an error envelope, the request was rejected before | `account` | string | Optional. Account name (set for individuals). | | `engName` | string | Optional. Populated only when request had `enrich: true`. | | `chineseName` | string | Optional. Populated only when `enrich: true`. | +| `name` | string | Optional. Bot/app display name from `apps.name` when the member's account ends with `.bot`. Mutually exclusive with `engName`/`chineseName`. | | `isOwner` | boolean | Optional. Populated only when `enrich: true`. | -| `sectName` | string | Optional. Populated only when `enrich: true` and entry is an org. | +| `orgName` | string | Optional. Org's display name (dept name preferred, sect name fallback). Populated only when `enrich: true` and entry is an org. | | `memberCount` | number | Optional. Populated only when `enrich: true` and entry is an org. | ```json @@ -1063,10 +1064,12 @@ See [Error envelope](#6-error-envelope-reference). Common errors: #### List Org Members -**Subject:** `chat.user.{account}.request.orgs.{orgID}.members` +**Subject:** `chat.user.{account}.request.orgs.{orgID}.{siteID}.members` **Reply subject:** auto-generated `_INBOX.>` (NATS request/reply) -The org ID is the second-to-last subject segment — there is no request body. `orgID` matches a user's `sectId` OR `deptId`; the response includes every user whose either field equals `orgID`. This mirrors the dept-aware org membership pipelines on the server side (a room may be added by sect-level or dept-level org and either form resolves through this endpoint). +- `{siteID}` selects which site's user directory to query for org membership. Each site has its own `users` collection, so the returned membership set is per-site. (When the caller is composing for a specific room, this is typically the room's origin `siteID` — the same value used for `member.list` — but the endpoint itself is org-scoped, not room-scoped.) + +The org ID is the third-from-last subject segment — there is no request body. `orgID` matches a user's `sectId` OR `deptId`; the response includes every user whose either field equals `orgID`. This mirrors the dept-aware org membership pipelines on the server side (a room may be added by sect-level or dept-level org and either form resolves through this endpoint). ##### Request body @@ -1124,6 +1127,86 @@ See [Error envelope](#6-error-envelope-reference). --- +#### Get Room App Tabs + +**Subject:** `chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs` +**Reply subject:** auto-generated `_INBOX.>` (NATS request/reply) + +- `{siteID}` must be the room's **origin `siteID`** (the site that owns the room), not the caller's own site. + +##### Request body + +Empty body (`{}` is tolerated). All inputs come from the subject. + +##### Success response + +| Field | Type | Notes | +|----------|----------------|-------| +| `apps` | array | Apps whose `channelTab.enabled` AND `channelTab.default` are both true, sorted by `channelTab.name asc`. Empty by default in DM/botDM rooms. | + +`RoomApp` fields: + +| Field | Type | Notes | +|-------------|---------------------|-------| +| `id` | string | `apps._id`. | +| `name` | string | `apps.channelTab.name`. | +| `tabUrl` | string | Computed: `SITE_URL`'s scheme/host/path-prefix + `apps.channelTab.url.default`'s path; `${roomId}` and `${siteId}` are substituted. Apps whose template URL is empty or unparseable are silently skipped. | +| `assistant` | object (optional) | `apps.assistant` subdocument if set. | +| `avatarUrl` | string (optional) | `apps.avatarUrl` if set. | + +##### Error response + +See [Error envelope](#6-error-envelope-reference). Common errors: `"not authorized to access this room's apps"` (caller is neither a room member nor a platform admin on the room's site), `"response payload exceeds maximum size"` (rare: response would exceed the NATS server's `max_payload`). + +##### Triggered events — success path + +`None — reply only.` + +##### Triggered events — error path + +`None — error returned only via the reply subject.` + +--- + +#### Get Room App Command Menu + +**Subject:** `chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu` +**Reply subject:** auto-generated `_INBOX.>` (NATS request/reply) + +- `{siteID}` must be the room's origin `siteID`. + +##### Request body + +Empty body (`{}` is tolerated). + +##### Success response + +| Field | Type | Notes | +|-----------------|-------------------------|-------| +| `appAssistants` | array | One entry per bot currently subscribed in the room whose owning app has `assistant.enabled=true`. Sorted by `name asc`. | + +`RoomAppAssistant` fields: + +| Field | Type | Notes | +|-------------|------------------|-------| +| `appName` | string | `apps.name`. | +| `name` | string | `apps.assistant.name` (the bot account). | +| `cmdBlocks` | array (optional) | Active command-menu blocks from `bot_cmd_menu` joined by name. Omitted/nil if no active menu exists for the bot. `CmdBlock` is recursive (`blocks` may contain further `CmdBlock`s) and may carry a `modal` object with `command` and `param`. | + +##### Error response + +Same envelope and sentinels as Get Room App Tabs. + +##### Triggered events — success path + +`None — reply only.` + +##### Triggered events — error path + +`None — error returned only via the reply subject.` + +--- + ### 3.2 history-service #### Common request fields (read RPCs) diff --git a/docs/superpowers/plans/2026-05-26-room-service-app-tabs-and-cmd-menu.md b/docs/superpowers/plans/2026-05-26-room-service-app-tabs-and-cmd-menu.md new file mode 100644 index 000000000..946f64cfa --- /dev/null +++ b/docs/superpowers/plans/2026-05-26-room-service-app-tabs-and-cmd-menu.md @@ -0,0 +1,2517 @@ +# Room-Service `app.tabs` + `app.cmd-menu` RPCs — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add two read-only client-facing NATS RPCs to `room-service` — `GetRoomAppTabs` (lists default-channel-tab apps with per-room rewritten tab URLs) and `GetRoomAppCommandMenu` (lists active command-menu blocks for every bot subscribed to the room). Both gated by room membership OR platform-admin role. + +**Architecture:** Two thin handlers in `room-service` + three new MongoDB store methods + a shared auth helper (`authorizeRoomAppRead`). URL rewrite preserves `SITE_URL` scheme/host/path-prefix and substitutes `${roomId}` / `${siteId}`. Response payload guarded by a per-handler bound sourced from `nc.MaxPayload()` and surfaced as a `errResponseTooLarge` sentinel. New `pkg/model` types: `User.Roles` + `IsPlatformAdmin`, `IsRoomMember`, `App.AvatarURL` + `App.ChannelTab`, `BotCmdMenu` / `CmdBlock` / `CmdModal`, and wire wrappers. + +**Tech Stack:** Go 1.25 · NATS core (request/reply) · MongoDB driver v2 (Find + Aggregate, no JetStream) · `go.uber.org/mock` · `stretchr/testify`. + +**Spec:** `docs/superpowers/specs/2026-05-26-room-service-app-tabs-and-cmd-menu-design.md` (commit `8662d05`). + +--- + +## File map + +| File | Action | +|------|--------| +| `pkg/subject/subject.go` | Add `RoomAppTabs`, `RoomAppTabsWildcard`, `RoomAppCmdMenu`, `RoomAppCmdMenuWildcard` | +| `pkg/subject/subject_test.go` | Add rows to `TestSubjectBuilders` | +| `pkg/model/user.go` | Add `Roles []string`, `PlatformRoleAdmin`, `IsPlatformAdmin` | +| `pkg/model/subscription.go` | Add `IsRoomMember` | +| `pkg/model/app.go` | Add `AvatarURL`, `ChannelTab`, `AppChannelTab`, `AppChannelTabURL`, `RoomApp`, `GetRoomAppTabsResponse`, `RoomAppAssistant`, `GetRoomAppCommandMenuResponse` | +| `pkg/model/botcmdmenu.go` | New file: `BotCmdMenu`, `CmdBlock`, `CmdModal` | +| `pkg/model/model_test.go` | Round-trip tests + `IsPlatformAdmin`/`IsRoomMember` unit tests | +| `room-service/store.go` | Add `RoomBotAppEntry`, `ListDefaultChannelTabApps`, `ListRoomBotApps`, `ListActiveCmdMenus` | +| `room-service/store_mongo.go` | Add `botCmdMenus` collection handle, three implementations, new compound indexes in `EnsureIndexes` | +| `room-service/integration_test.go` | Add `TestMongoStore_ListDefaultChannelTabApps`, `_ListRoomBotApps`, `_ListActiveCmdMenus`, `_ListActiveCmdMenus_Empty`, `_EnsureIndexes_NewCompoundIndexes` | +| `room-service/mock_store_test.go` | Regenerated via `make generate SERVICE=room-service` (don't hand-edit) | +| `room-service/helper.go` | Add `errAppAccessDenied`, `errResponseTooLarge`, `marshalBounded`, `replyBoundedJSON`; extend `sanitizeError` | +| `room-service/handler.go` | Add `siteURL`, `maxResponseBytes` fields; `authorizeRoomAppRead`, `buildTabURL`, `handleGetRoomAppTabs`, `natsGetRoomAppTabs`, `handleGetRoomAppCommandMenu`, `natsGetRoomAppCommandMenu`; extend `RegisterCRUD` | +| `room-service/handler_test.go` | Tests for `marshalBounded`, `authorizeRoomAppRead`, `buildTabURL`, both handlers | +| `room-service/main.go` | Add `SiteURL string` config field, validate at startup, pass `siteURL` and `nc.MaxPayload()` into `NewHandler` | +| `room-service/deploy/docker-compose.yml` | Add `SITE_URL` to the `room-service` env | +| `docs/client-api.md` | Document the two new RPCs under §3.1 (room-service) | + +--- + +## Task 0: Add subject builders + +**Files:** +- Modify: `pkg/subject/subject.go` +- Modify: `pkg/subject/subject_test.go` + +- [ ] **Step 1: Write failing tests** + +Append four rows to the `tests` table inside `TestSubjectBuilders` in `pkg/subject/subject_test.go` (the existing table starts around line 11; add inside it): + +```go +{"RoomAppTabs", subject.RoomAppTabs("alice", "r1", "site-a"), + "chat.user.alice.request.room.r1.site-a.app.tabs"}, +{"RoomAppTabsWildcard", subject.RoomAppTabsWildcard("site-a"), + "chat.user.*.request.room.*.site-a.app.tabs"}, +{"RoomAppCmdMenu", subject.RoomAppCmdMenu("alice", "r1", "site-a"), + "chat.user.alice.request.room.r1.site-a.app.cmd-menu"}, +{"RoomAppCmdMenuWildcard", subject.RoomAppCmdMenuWildcard("site-a"), + "chat.user.*.request.room.*.site-a.app.cmd-menu"}, +``` + +Add two new standalone tests at the end of the file (after the table) covering the round-trip parse via the shared `ParseUserRoomSubject`: + +```go +func TestRoomAppTabs_ParseUserRoomSubject(t *testing.T) { + subj := subject.RoomAppTabs("alice", "r1", "site-a") + account, roomID, ok := subject.ParseUserRoomSubject(subj) + assert.True(t, ok) + assert.Equal(t, "alice", account) + assert.Equal(t, "r1", roomID) +} + +func TestRoomAppCmdMenu_ParseUserRoomSubject(t *testing.T) { + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + account, roomID, ok := subject.ParseUserRoomSubject(subj) + assert.True(t, ok) + assert.Equal(t, "alice", account) + assert.Equal(t, "r1", roomID) +} +``` + +- [ ] **Step 2: Run tests — expect FAIL** + +Run: `make test SERVICE=pkg/subject` + +Expected: FAIL with "undefined: subject.RoomAppTabs" / "RoomAppTabsWildcard" / "RoomAppCmdMenu" / "RoomAppCmdMenuWildcard". + +- [ ] **Step 3: Implement the builders** + +Append to `pkg/subject/subject.go` (after `MuteToggleWildcard` around the existing room-scoped builders block): + +```go +// RoomAppTabs returns the concrete subject for the GetRoomAppTabs RPC. +// Pair with RoomAppTabsWildcard for room-service's QueueSubscribe. +func RoomAppTabs(account, roomID, siteID string) string { + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.tabs", account, roomID, siteID) +} + +// RoomAppTabsWildcard is the per-site subscription pattern for the +// GetRoomAppTabs RPC. +func RoomAppTabsWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.tabs", siteID) +} + +// RoomAppCmdMenu returns the concrete subject for the +// GetRoomAppCommandMenu RPC. +func RoomAppCmdMenu(account, roomID, siteID string) string { + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.cmd-menu", account, roomID, siteID) +} + +// RoomAppCmdMenuWildcard is the per-site subscription pattern for the +// GetRoomAppCommandMenu RPC. +func RoomAppCmdMenuWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.cmd-menu", siteID) +} +``` + +- [ ] **Step 4: Run tests — expect PASS** + +Run: `make test SERVICE=pkg/subject` + +Expected: PASS, all rows green. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/subject/subject.go pkg/subject/subject_test.go +git commit -m "feat(subject): add RoomAppTabs and RoomAppCmdMenu builders" +``` + +--- + +## Task 1: Add `User.Roles` + `IsPlatformAdmin` to `pkg/model` + +**Files:** +- Modify: `pkg/model/user.go` +- Modify: `pkg/model/model_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `pkg/model/model_test.go` (after the existing User tests around line 40): + +```go +func TestUserJSON_WithRoles(t *testing.T) { + u := model.User{ + ID: "u1", + Account: "alice", + SiteID: "site-a", + Roles: []string{"user", "admin"}, + } + roundTrip(t, &u, &model.User{}) +} + +func TestUserJSON_RolesOmittedWhenEmpty(t *testing.T) { + u := model.User{ID: "u1", Account: "alice", SiteID: "site-a"} + data, err := json.Marshal(&u) + require.NoError(t, err) + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) + _, has := raw["roles"] + assert.False(t, has, "nil Roles must be omitted from JSON") +} + +func TestPlatformRoleAdminConstant(t *testing.T) { + assert.Equal(t, "admin", model.PlatformRoleAdmin) +} + +func TestIsPlatformAdmin(t *testing.T) { + tests := []struct { + name string + u *model.User + want bool + }{ + {"nil receiver", nil, false}, + {"nil roles", &model.User{Account: "alice"}, false}, + {"empty roles", &model.User{Account: "alice", Roles: []string{}}, false}, + {"user role only", &model.User{Account: "alice", Roles: []string{"user"}}, false}, + {"admin role present", &model.User{Account: "alice", Roles: []string{"admin"}}, true}, + {"admin among many", &model.User{Account: "alice", Roles: []string{"user", "admin", "auditor"}}, true}, + {"case-sensitive (Admin not admin)", &model.User{Account: "alice", Roles: []string{"Admin"}}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, model.IsPlatformAdmin(tt.u)) + }) + } +} +``` + +- [ ] **Step 2: Run tests — expect FAIL** + +Run: `make test SERVICE=pkg/model` + +Expected: FAIL — undefined `model.PlatformRoleAdmin`, `model.IsPlatformAdmin`, missing `Roles` field on `User`. + +- [ ] **Step 3: Implement** + +Edit `pkg/model/user.go`. Replace the file contents with: + +```go +package model + +type User struct { + ID string `json:"id" bson:"_id"` + Account string `json:"account" bson:"account"` + SiteID string `json:"siteId" bson:"siteId"` + SectID string `json:"sectId" bson:"sectId"` + SectName string `json:"sectName" bson:"sectName"` + SectTCName string `json:"sectTCName" bson:"sectTCName"` + DeptID string `json:"deptId" bson:"deptId"` + DeptName string `json:"deptName" bson:"deptName"` + DeptTCName string `json:"deptTCName" bson:"deptTCName"` + EngName string `json:"engName" bson:"engName"` + ChineseName string `json:"chineseName" bson:"chineseName"` + EmployeeID string `json:"employeeId" bson:"employeeId"` + Roles []string `json:"roles,omitempty" bson:"roles,omitempty"` +} + +// PlatformRoleAdmin is the platform-admin role string carried in User.Roles. +const PlatformRoleAdmin = "admin" + +// IsPlatformAdmin reports whether u holds the platform admin role. +// Returns false for nil receivers and for users without the role. +func IsPlatformAdmin(u *User) bool { + if u == nil { + return false + } + for _, r := range u.Roles { + if r == PlatformRoleAdmin { + return true + } + } + return false +} +``` + +- [ ] **Step 4: Run tests — expect PASS** + +Run: `make test SERVICE=pkg/model` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/model/user.go pkg/model/model_test.go +git commit -m "feat(model): add User.Roles and IsPlatformAdmin helper" +``` + +--- + +## Task 2: Add `IsRoomMember` helper to `pkg/model` + +**Files:** +- Modify: `pkg/model/subscription.go` +- Modify: `pkg/model/model_test.go` + +- [ ] **Step 1: Write failing test** + +Append to `pkg/model/model_test.go`: + +```go +func TestIsRoomMember(t *testing.T) { + assert.False(t, model.IsRoomMember(nil), "nil sub is not a member") + assert.True(t, model.IsRoomMember(&model.Subscription{RoomID: "r1"}), "non-nil sub is a member") +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=pkg/model` + +Expected: FAIL — undefined `model.IsRoomMember`. + +- [ ] **Step 3: Implement** + +Append to `pkg/model/subscription.go` (after the existing types, before any blank-line EOF): + +```go +// IsRoomMember reports whether sub represents an active membership. +// Returns false for nil so callers can pass the result of a store lookup +// that returned (nil, ErrSubscriptionNotFound) — the caller is expected +// to have already classified the error and set sub to nil on not-found. +func IsRoomMember(sub *Subscription) bool { + return sub != nil +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=pkg/model` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/model/subscription.go pkg/model/model_test.go +git commit -m "feat(model): add IsRoomMember predicate" +``` + +--- + +## Task 3: Extend `App` with `AvatarURL` and `ChannelTab` + +**Files:** +- Modify: `pkg/model/app.go` +- Modify: `pkg/model/model_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `pkg/model/model_test.go`: + +```go +func TestAppRoundtrip_WithChannelTabAndAvatar(t *testing.T) { + a := model.App{ + ID: "app1", + Name: "Calendar", + AvatarURL: "https://cdn.example.com/avatars/calendar.png", + Assistant: &model.AppAssistant{Enabled: true, Name: "calendar.bot"}, + ChannelTab: &model.AppChannelTab{ + Enabled: true, + Default: true, + Name: "Calendar", + URL: model.AppChannelTabURL{ + Default: "https://upstream.example.com/calendar/${roomId}/${siteId}/index", + }, + }, + } + var dst model.App + roundTrip(t, &a, &dst) + require.NotNil(t, dst.ChannelTab) + assert.True(t, dst.ChannelTab.Enabled) + assert.True(t, dst.ChannelTab.Default) + assert.Equal(t, "Calendar", dst.ChannelTab.Name) + assert.Equal(t, "https://upstream.example.com/calendar/${roomId}/${siteId}/index", + dst.ChannelTab.URL.Default) + assert.Equal(t, "https://cdn.example.com/avatars/calendar.png", dst.AvatarURL) +} + +func TestAppChannelTabRoundtrip(t *testing.T) { + tab := model.AppChannelTab{ + Enabled: true, + Default: false, + Name: "Notes", + URL: model.AppChannelTabURL{Default: "https://upstream/notes"}, + } + var dst model.AppChannelTab + roundTrip(t, &tab, &dst) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=pkg/model` + +Expected: FAIL — undefined `AppChannelTab`, `AppChannelTabURL`, missing fields on `App`. + +- [ ] **Step 3: Implement** + +Edit `pkg/model/app.go`. Replace the existing `App` struct and add the two new nested types. The full file should read: + +```go +package model + +// App is a read-only view of the apps collection (provisioning is upstream). +type App struct { + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + AvatarURL string `json:"avatarUrl,omitempty" bson:"avatarUrl,omitempty"` + Assistant *AppAssistant `json:"assistant,omitempty" bson:"assistant,omitempty"` + ChannelTab *AppChannelTab `json:"channelTab,omitempty" bson:"channelTab,omitempty"` + Sponsors []AppSponsor `json:"sponsors,omitempty" bson:"sponsors,omitempty"` +} + +// AppAssistant: Name is the bot user account (".bot" suffix); botDM requires Enabled==true. +type AppAssistant struct { + Enabled bool `json:"enabled" bson:"enabled"` + Name string `json:"name" bson:"name"` + SettingsURL string `json:"settingsUrl,omitempty" bson:"settingsUrl,omitempty"` +} + +// AppChannelTab describes a tab that can be embedded into channel rooms. +// Default==true marks tabs that appear by default in every channel. +type AppChannelTab struct { + Enabled bool `json:"enabled" bson:"enabled"` + Default bool `json:"default" bson:"default"` + Name string `json:"name" bson:"name"` + URL AppChannelTabURL `json:"url" bson:"url"` +} + +// AppChannelTabURL holds the URL template. Default is the canonical form +// with literal ${roomId} / ${siteId} placeholders that room-service +// substitutes when building per-room tab URLs. +type AppChannelTabURL struct { + Default string `json:"default" bson:"default"` +} + +type AppSponsor struct { + Name string `json:"name" bson:"name"` + Phone string `json:"phone" bson:"phone"` +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=pkg/model` + +Expected: PASS (including the existing `TestAppRoundtrip` and `TestAppAssistantDisabledRoundtrip`, which must still pass — they don't set the new fields). + +- [ ] **Step 5: Commit** + +```bash +git add pkg/model/app.go pkg/model/model_test.go +git commit -m "feat(model): add App.AvatarURL and App.ChannelTab schema" +``` + +--- + +## Task 4: Add bot command-menu types + +**Files:** +- Create: `pkg/model/botcmdmenu.go` +- Modify: `pkg/model/model_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `pkg/model/model_test.go`: + +```go +func TestBotCmdMenuRoundtrip(t *testing.T) { + m := model.BotCmdMenu{ + ID: "bcm1", + Name: "weather.bot", + ActiveStatus: true, + CmdBlocks: []model.CmdBlock{ + {Text: "/weather", ActionType: "command", Payload: "weather"}, + }, + } + var dst model.BotCmdMenu + roundTrip(t, &m, &dst) + require.Len(t, dst.CmdBlocks, 1) + assert.Equal(t, "/weather", dst.CmdBlocks[0].Text) +} + +func TestBotCmdMenuRoundtrip_Inactive(t *testing.T) { + m := model.BotCmdMenu{ + ID: "bcm2", + Name: "weather.bot", + ActiveStatus: false, + } + var dst model.BotCmdMenu + roundTrip(t, &m, &dst) + assert.False(t, dst.ActiveStatus) + assert.Nil(t, dst.CmdBlocks) +} + +func TestCmdBlockRoundtrip_Recursive(t *testing.T) { + block := model.CmdBlock{ + Text: "menu", + ActionType: "open", + Description: "open the menu", + Modal: &model.CmdModal{ + Command: "menu.open", + Param: "weather", + }, + Blocks: []model.CmdBlock{ + {Text: "today", Payload: "today"}, + {Text: "tomorrow", Payload: "tomorrow", Blocks: []model.CmdBlock{ + {Text: "morning", Payload: "tomorrow.am"}, + }}, + }, + } + var dst model.CmdBlock + roundTrip(t, &block, &dst) + require.NotNil(t, dst.Modal) + assert.Equal(t, "menu.open", dst.Modal.Command) + assert.Equal(t, "weather", dst.Modal.Param) + require.Len(t, dst.Blocks, 2) + require.Len(t, dst.Blocks[1].Blocks, 1) + assert.Equal(t, "morning", dst.Blocks[1].Blocks[0].Text) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=pkg/model` + +Expected: FAIL — undefined `model.BotCmdMenu`, `model.CmdBlock`, `model.CmdModal`. + +- [ ] **Step 3: Implement — create new file** + +Create `pkg/model/botcmdmenu.go`: + +```go +package model + +// BotCmdMenu is a row in the bot_cmd_menu collection. Name matches an +// AppAssistant.Name (the bot account) and joins back to the owning App +// via that field. ActiveStatus gates whether the menu is currently +// exposed to clients. +type BotCmdMenu struct { + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + ActiveStatus bool `json:"activeStatus" bson:"activeStatus"` + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty" bson:"cmdBlocks,omitempty"` +} + +// CmdBlock is the recursive building block of a bot command menu. A +// block either renders directly (Text+ActionType+Payload), opens a +// modal (Modal), or groups nested blocks (Blocks). Fields are +// optional so the schema can evolve without breaking the wire +// contract. +type CmdBlock struct { + Text string `json:"text,omitempty" bson:"text,omitempty"` + ActionType string `json:"actionType,omitempty" bson:"actionType,omitempty"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + Payload string `json:"payload,omitempty" bson:"payload,omitempty"` + Modal *CmdModal `json:"modal,omitempty" bson:"modal,omitempty"` + Blocks []CmdBlock `json:"blocks,omitempty" bson:"blocks,omitempty"` +} + +// CmdModal carries the slash-style command + param a modal-triggering +// block invokes. CmdModal does NOT nest its own blocks — recursive +// rendering happens via CmdBlock.Blocks on the enclosing CmdBlock. +type CmdModal struct { + Command string `json:"command,omitempty" bson:"command,omitempty"` + Param string `json:"param,omitempty" bson:"param,omitempty"` +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=pkg/model` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/model/botcmdmenu.go pkg/model/model_test.go +git commit -m "feat(model): add BotCmdMenu, CmdBlock, CmdModal types" +``` + +--- + +## Task 5: Add wire-format response wrappers + +**Files:** +- Modify: `pkg/model/app.go` +- Modify: `pkg/model/model_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `pkg/model/model_test.go`: + +```go +func TestGetRoomAppTabsResponseRoundtrip(t *testing.T) { + src := model.GetRoomAppTabsResponse{ + Apps: []model.RoomApp{ + { + ID: "app1", + Name: "Calendar", + TabURL: "https://chat.example.com/calendar/r1/site-a/index", + Assistant: &model.AppAssistant{Enabled: true, Name: "cal.bot"}, + AvatarURL: "https://cdn/cal.png", + }, + }, + } + var dst model.GetRoomAppTabsResponse + roundTrip(t, &src, &dst) + require.Len(t, dst.Apps, 1) + assert.Equal(t, "Calendar", dst.Apps[0].Name) +} + +func TestGetRoomAppTabsResponse_EmptyIsArrayNotNull(t *testing.T) { + src := model.GetRoomAppTabsResponse{Apps: []model.RoomApp{}} + data, err := json.Marshal(&src) + require.NoError(t, err) + assert.Contains(t, string(data), `"apps":[]`) + assert.NotContains(t, string(data), `"apps":null`) +} + +func TestGetRoomAppCommandMenuResponseRoundtrip(t *testing.T) { + src := model.GetRoomAppCommandMenuResponse{ + AppAssistants: []model.RoomAppAssistant{ + { + AppName: "Weather Bot", + Name: "weather.bot", + CmdBlocks: []model.CmdBlock{ + {Text: "/forecast", Payload: "forecast"}, + }, + }, + }, + } + var dst model.GetRoomAppCommandMenuResponse + roundTrip(t, &src, &dst) + require.Len(t, dst.AppAssistants, 1) + assert.Equal(t, "weather.bot", dst.AppAssistants[0].Name) +} + +func TestGetRoomAppCommandMenuResponse_EmptyIsArrayNotNull(t *testing.T) { + src := model.GetRoomAppCommandMenuResponse{ + AppAssistants: []model.RoomAppAssistant{}, + } + data, err := json.Marshal(&src) + require.NoError(t, err) + assert.Contains(t, string(data), `"appAssistants":[]`) + assert.NotContains(t, string(data), `"appAssistants":null`) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=pkg/model` + +Expected: FAIL — undefined `RoomApp`, `RoomAppAssistant`, `GetRoomAppTabsResponse`, `GetRoomAppCommandMenuResponse`. + +- [ ] **Step 3: Implement** + +Append to `pkg/model/app.go` (after the existing types): + +```go +// RoomApp is a single entry in GetRoomAppTabsResponse.Apps — derived +// from an apps document with the per-room tabUrl substituted in. +type RoomApp struct { + ID string `json:"id"` + Name string `json:"name"` // = apps.channelTab.name + TabURL string `json:"tabUrl"` // computed (scheme+host+path-prefix from SITE_URL, ${roomId}/${siteId} substituted) + Assistant *AppAssistant `json:"assistant,omitempty"` + AvatarURL string `json:"avatarUrl,omitempty"` +} + +// GetRoomAppTabsResponse is the response body for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs RPC. +type GetRoomAppTabsResponse struct { + Apps []RoomApp `json:"apps"` +} + +// RoomAppAssistant is a single entry in +// GetRoomAppCommandMenuResponse.AppAssistants. +type RoomAppAssistant struct { + AppName string `json:"appName"` // = apps.name + Name string `json:"name"` // = apps.assistant.name (bot account) + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty"` +} + +// GetRoomAppCommandMenuResponse is the response body for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu RPC. +type GetRoomAppCommandMenuResponse struct { + AppAssistants []RoomAppAssistant `json:"appAssistants"` +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=pkg/model` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add pkg/model/app.go pkg/model/model_test.go +git commit -m "feat(model): add wire types for GetRoomAppTabs and GetRoomAppCommandMenu" +``` + +--- + +## Task 6: Extend `RoomStore` interface + regenerate mocks + +**Files:** +- Modify: `room-service/store.go` +- Modify: `room-service/mock_store_test.go` (regenerated, do not hand-edit) + +- [ ] **Step 1: Edit `room-service/store.go`** + +Add the `RoomBotAppEntry` type near the other store-internal types (near `RoomCounts` / `ReadReceiptRow`), and add three new methods to the `RoomStore` interface. Insert after the existing read-side methods (e.g. after `GetApp`): + +```go +// RoomBotAppEntry pairs an assistant's bot account with its owning +// app name — the joined output of ListRoomBotApps. +type RoomBotAppEntry struct { + AssistantName string `bson:"assistantName"` + AppName string `bson:"appName"` +} +``` + +Append the following methods to the `RoomStore` interface body: + +```go +// ListDefaultChannelTabApps returns apps whose channelTab.enabled AND +// channelTab.default are both true, sorted by channelTab.name asc. +// Projection: _id, avatarUrl, assistant, channelTab. Empty result is +// ([], nil). +ListDefaultChannelTabApps(ctx context.Context) ([]model.App, error) + +// ListRoomBotApps returns one entry per bot subscribed to roomID, +// joined with the owning app via assistant.name == u.account. Only +// apps with assistant.enabled=true are emitted. Empty result is +// ([], nil); result order is assistantName asc. +ListRoomBotApps(ctx context.Context, roomID string) ([]RoomBotAppEntry, error) + +// ListActiveCmdMenus returns bot_cmd_menu documents where +// activeStatus is true AND name IN assistantNames, sorted by name asc. +// Returns ([], nil) when assistantNames is empty (skips the query). +ListActiveCmdMenus(ctx context.Context, assistantNames []string) ([]model.BotCmdMenu, error) +``` + +- [ ] **Step 2: Regenerate mocks** + +Run: `make generate SERVICE=room-service` + +Expected: `room-service/mock_store_test.go` is rewritten with the three new methods. Do NOT hand-edit; if regen fails, fix the interface in `store.go` and re-run. + +- [ ] **Step 3: Confirm package compiles** + +Run: `make test SERVICE=room-service` + +Expected: ALL EXISTING TESTS STILL PASS. (The new methods aren't called yet; this confirms the regen produced valid mock code and the interface is well-formed.) If a test fails because some helper still references the old mock surface, that's a regen problem — re-run `make generate SERVICE=room-service`. + +- [ ] **Step 4: Commit** + +```bash +git add room-service/store.go room-service/mock_store_test.go +git commit -m "feat(room-service): add RoomBotAppEntry and three new store methods" +``` + +--- + +## Task 7: Implement `ListDefaultChannelTabApps` + integration test + +**Files:** +- Modify: `room-service/store_mongo.go` +- Modify: `room-service/integration_test.go` + +- [ ] **Step 1: Write failing integration test** + +Append to `room-service/integration_test.go`: + +```go +func TestMongoStore_ListDefaultChannelTabApps(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + ctx := context.Background() + + apps := []any{ + bson.M{ + "_id": "app-z", "name": "Zeta", + "channelTab": bson.M{ + "enabled": true, "default": true, "name": "Zeta", + "url": bson.M{"default": "https://upstream/z"}, + }, + }, + bson.M{ + "_id": "app-a", "name": "Alpha", + "channelTab": bson.M{ + "enabled": true, "default": true, "name": "Alpha", + "url": bson.M{"default": "https://upstream/a"}, + }, + }, + bson.M{ + "_id": "app-disabled", "name": "Disabled", + "channelTab": bson.M{ + "enabled": false, "default": true, "name": "Disabled", + "url": bson.M{"default": "https://upstream/d"}, + }, + }, + bson.M{"_id": "app-notabs", "name": "NoTabs"}, + } + _, err := db.Collection("apps").InsertMany(ctx, apps) + require.NoError(t, err) + + got, err := store.ListDefaultChannelTabApps(ctx) + require.NoError(t, err) + require.Len(t, got, 2) + assert.Equal(t, "app-a", got[0].ID, "expected Alpha first by channelTab.name asc") + assert.Equal(t, "app-z", got[1].ID) + assert.Equal(t, "Alpha", got[0].ChannelTab.Name) + // Projection excludes app.name (response uses channelTab.name). + assert.Empty(t, got[0].Name) +} + +func TestMongoStore_ListDefaultChannelTabApps_Empty(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + got, err := store.ListDefaultChannelTabApps(context.Background()) + require.NoError(t, err) + assert.Empty(t, got) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test-integration SERVICE=room-service` + +Expected: FAIL — undefined method `ListDefaultChannelTabApps` on `*MongoStore`. + +- [ ] **Step 3: Implement** + +Append to `room-service/store_mongo.go` (after the existing read-side methods, before the bottom of the file): + +```go +// ListDefaultChannelTabApps returns apps whose channelTab.enabled +// AND channelTab.default are both true, sorted by channelTab.name asc. +// Projection drops app.name (response uses channelTab.name). +func (s *MongoStore) ListDefaultChannelTabApps(ctx context.Context) ([]model.App, error) { + opts := options.Find(). + SetSort(bson.D{{Key: "channelTab.name", Value: 1}}). + SetProjection(bson.M{ + "_id": 1, + "avatarUrl": 1, + "assistant": 1, + "channelTab": 1, + }) + cursor, err := s.apps.Find(ctx, bson.M{ + "channelTab.enabled": true, + "channelTab.default": true, + }, opts) + if err != nil { + return nil, fmt.Errorf("list default channel-tab apps: %w", err) + } + defer cursor.Close(ctx) + apps := make([]model.App, 0) + if err := cursor.All(ctx, &apps); err != nil { + return nil, fmt.Errorf("decode default channel-tab apps: %w", err) + } + return apps, nil +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test-integration SERVICE=room-service` + +Expected: PASS for `TestMongoStore_ListDefaultChannelTabApps` and `..._Empty`. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/store_mongo.go room-service/integration_test.go +git commit -m "feat(room-service): implement ListDefaultChannelTabApps" +``` + +--- + +## Task 8: Implement `ListRoomBotApps` aggregation + integration test + +**Files:** +- Modify: `room-service/store_mongo.go` +- Modify: `room-service/integration_test.go` + +- [ ] **Step 1: Write failing integration test** + +Append to `room-service/integration_test.go`: + +```go +func TestMongoStore_ListRoomBotApps(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + ctx := context.Background() + + // Apps: one enabled bot, one disabled, one without assistant. + _, err := db.Collection("apps").InsertMany(ctx, []any{ + bson.M{"_id": "appA", "name": "Weather", + "assistant": bson.M{"enabled": true, "name": "weather.bot"}}, + bson.M{"_id": "appB", "name": "Stocks", + "assistant": bson.M{"enabled": false, "name": "stocks.bot"}}, + bson.M{"_id": "appC", "name": "NoBot"}, + }) + require.NoError(t, err) + + // Subscriptions: roomA has 1 bot (weather, enabled) + 1 disabled bot + // (stocks, but assistant.enabled=false should drop it) + 1 human; + // roomB has 1 different bot. + _, err = db.Collection("subscriptions").InsertMany(ctx, []any{ + bson.M{"_id": "s1", "roomId": "roomA", + "u": bson.M{"_id": "ub1", "account": "weather.bot", "isBot": true}}, + bson.M{"_id": "s2", "roomId": "roomA", + "u": bson.M{"_id": "ub2", "account": "stocks.bot", "isBot": true}}, + bson.M{"_id": "s3", "roomId": "roomA", + "u": bson.M{"_id": "uh1", "account": "alice", "isBot": false}}, + bson.M{"_id": "s4", "roomId": "roomB", + "u": bson.M{"_id": "ub3", "account": "other.bot", "isBot": true}}, + }) + require.NoError(t, err) + + got, err := store.ListRoomBotApps(ctx, "roomA") + require.NoError(t, err) + require.Len(t, got, 1, "only enabled assistant should join in") + assert.Equal(t, "weather.bot", got[0].AssistantName) + assert.Equal(t, "Weather", got[0].AppName) +} + +func TestMongoStore_ListRoomBotApps_Empty(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + got, err := store.ListRoomBotApps(context.Background(), "ghost-room") + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestMongoStore_ListRoomBotApps_UniqueIndexProtectsAgainstDupes(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + require.NoError(t, store.EnsureIndexes(context.Background())) + ctx := context.Background() + + _, err := db.Collection("apps").InsertOne(ctx, bson.M{ + "_id": "appA", "name": "Weather", + "assistant": bson.M{"enabled": true, "name": "weather.bot"}, + }) + require.NoError(t, err) + _, err = db.Collection("subscriptions").InsertOne(ctx, bson.M{ + "_id": "s1", "roomId": "roomA", + "u": bson.M{"_id": "ub1", "account": "weather.bot", "isBot": true}, + }) + require.NoError(t, err) + // Duplicate (roomId, u.account) must be rejected by the unique index. + _, err = db.Collection("subscriptions").InsertOne(ctx, bson.M{ + "_id": "s2", "roomId": "roomA", + "u": bson.M{"_id": "ub1b", "account": "weather.bot", "isBot": true}, + }) + require.Error(t, err) + assert.True(t, mongo.IsDuplicateKeyError(err), "expected duplicate-key error from (roomId, u.account) unique index") +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test-integration SERVICE=room-service` + +Expected: FAIL — undefined `ListRoomBotApps`. + +- [ ] **Step 3: Implement** + +Append to `room-service/store_mongo.go`: + +```go +// ListRoomBotApps returns one entry per bot subscribed to roomID, +// joined with the owning app via assistant.name == u.account. Drops +// apps whose assistant.enabled is false. Output is sorted by +// assistantName asc for deterministic test/client ordering. +func (s *MongoStore) ListRoomBotApps(ctx context.Context, roomID string) ([]RoomBotAppEntry, error) { + pipeline := mongo.Pipeline{ + {{Key: "$match", Value: bson.M{"roomId": roomID, "u.isBot": true}}}, + {{Key: "$lookup", Value: bson.M{ + "from": "apps", + "let": bson.M{"acct": "$u.account"}, + "pipeline": bson.A{ + bson.M{"$match": bson.M{"$expr": bson.M{"$and": bson.A{ + bson.M{"$eq": bson.A{"$assistant.enabled", true}}, + bson.M{"$eq": bson.A{"$assistant.name", "$$acct"}}, + }}}}, + bson.M{"$project": bson.M{ + "_id": 0, + "assistantName": "$assistant.name", + "appName": "$name", + }}, + }, + "as": "app", + }}}, + {{Key: "$unwind", Value: "$app"}}, + {{Key: "$replaceRoot", Value: bson.M{"newRoot": "$app"}}}, + {{Key: "$sort", Value: bson.D{{Key: "assistantName", Value: 1}}}}, + } + cursor, err := s.subscriptions.Aggregate(ctx, pipeline) + if err != nil { + return nil, fmt.Errorf("list room bot apps for %q: %w", roomID, err) + } + defer cursor.Close(ctx) + entries := make([]RoomBotAppEntry, 0) + if err := cursor.All(ctx, &entries); err != nil { + return nil, fmt.Errorf("decode room bot apps for %q: %w", roomID, err) + } + return entries, nil +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test-integration SERVICE=room-service` + +Expected: PASS for the three new tests. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/store_mongo.go room-service/integration_test.go +git commit -m "feat(room-service): implement ListRoomBotApps aggregation" +``` + +--- + +## Task 9: Implement `ListActiveCmdMenus` + integration test + +**Files:** +- Modify: `room-service/store_mongo.go` +- Modify: `room-service/integration_test.go` + +- [ ] **Step 1: Write failing integration test** + +Append to `room-service/integration_test.go`: + +```go +func TestMongoStore_ListActiveCmdMenus(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + ctx := context.Background() + + _, err := db.Collection("bot_cmd_menu").InsertMany(ctx, []any{ + bson.M{"_id": "m1", "name": "weather.bot", "activeStatus": true, + "cmdBlocks": []bson.M{{"text": "/forecast"}}}, + bson.M{"_id": "m2", "name": "weather.bot", "activeStatus": false, + "cmdBlocks": []bson.M{{"text": "/old"}}}, + bson.M{"_id": "m3", "name": "stocks.bot", "activeStatus": true, + "cmdBlocks": []bson.M{{"text": "/quote"}}}, + bson.M{"_id": "m4", "name": "other.bot", "activeStatus": true, + "cmdBlocks": []bson.M{{"text": "/x"}}}, + }) + require.NoError(t, err) + + got, err := store.ListActiveCmdMenus(ctx, []string{"weather.bot", "stocks.bot"}) + require.NoError(t, err) + require.Len(t, got, 2, "expect only the two active matching rows") + assert.Equal(t, "stocks.bot", got[0].Name, "sorted by name asc") + assert.Equal(t, "weather.bot", got[1].Name) + require.Len(t, got[1].CmdBlocks, 1) + assert.Equal(t, "/forecast", got[1].CmdBlocks[0].Text) + // Projection drops _id and activeStatus. + assert.Empty(t, got[1].ID) + assert.False(t, got[1].ActiveStatus) +} + +func TestMongoStore_ListActiveCmdMenus_EmptyInput(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + got, err := store.ListActiveCmdMenus(context.Background(), nil) + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestMongoStore_ListActiveCmdMenus_NoMatches(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + got, err := store.ListActiveCmdMenus(context.Background(), []string{"unknown.bot"}) + require.NoError(t, err) + assert.Empty(t, got) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test-integration SERVICE=room-service` + +Expected: FAIL — undefined `ListActiveCmdMenus`. + +- [ ] **Step 3: Implement** + +Add the `botCmdMenus` collection handle to `MongoStore`. Locate the struct and constructor near the top of `room-service/store_mongo.go`. Replace them with: + +```go +type MongoStore struct { + rooms *mongo.Collection + subscriptions *mongo.Collection + threadSubscriptions *mongo.Collection + roomMembers *mongo.Collection + users *mongo.Collection + apps *mongo.Collection + botCmdMenus *mongo.Collection +} + +func NewMongoStore(db *mongo.Database) *MongoStore { + return &MongoStore{ + rooms: db.Collection("rooms"), + subscriptions: db.Collection("subscriptions"), + threadSubscriptions: db.Collection("thread_subscriptions"), + roomMembers: db.Collection("room_members"), + users: db.Collection("users"), + apps: db.Collection("apps"), + botCmdMenus: db.Collection("bot_cmd_menu"), + } +} +``` + +Then append the new method: + +```go +// ListActiveCmdMenus returns bot_cmd_menu documents where +// activeStatus is true AND name IN assistantNames, sorted by name asc. +// Returns ([], nil) when assistantNames is empty (skips the query). +// Upstream invariant: at most one active row per name (out-of-scope +// to enforce via a partial unique index on the writer side). +func (s *MongoStore) ListActiveCmdMenus(ctx context.Context, assistantNames []string) ([]model.BotCmdMenu, error) { + if len(assistantNames) == 0 { + return []model.BotCmdMenu{}, nil + } + opts := options.Find(). + SetSort(bson.D{{Key: "name", Value: 1}}). + SetProjection(bson.M{ + "_id": 0, + "name": 1, + "cmdBlocks": 1, + }) + cursor, err := s.botCmdMenus.Find(ctx, bson.M{ + "activeStatus": true, + "name": bson.M{"$in": assistantNames}, + }, opts) + if err != nil { + return nil, fmt.Errorf("list active cmd menus: %w", err) + } + defer cursor.Close(ctx) + menus := make([]model.BotCmdMenu, 0) + if err := cursor.All(ctx, &menus); err != nil { + return nil, fmt.Errorf("decode active cmd menus: %w", err) + } + return menus, nil +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test-integration SERVICE=room-service` + +Expected: PASS for the three new tests. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/store_mongo.go room-service/integration_test.go +git commit -m "feat(room-service): implement ListActiveCmdMenus" +``` + +--- + +## Task 10: Add new compound indexes to `EnsureIndexes` + +**Files:** +- Modify: `room-service/store_mongo.go` +- Modify: `room-service/integration_test.go` + +- [ ] **Step 1: Write failing integration test** + +Append to `room-service/integration_test.go`: + +```go +func TestMongoStore_EnsureIndexes_NewCompoundIndexes(t *testing.T) { + db := setupMongo(t) + store := NewMongoStore(db) + require.NoError(t, store.EnsureIndexes(context.Background())) + + type idxCheck struct { + coll string + wantKeys bson.D + } + checks := []idxCheck{ + {"apps", bson.D{ + {Key: "channelTab.default", Value: int32(1)}, + {Key: "channelTab.enabled", Value: int32(1)}, + {Key: "channelTab.name", Value: int32(1)}, + }}, + {"subscriptions", bson.D{ + {Key: "roomId", Value: int32(1)}, + {Key: "u.isBot", Value: int32(1)}, + }}, + {"bot_cmd_menu", bson.D{ + {Key: "activeStatus", Value: int32(1)}, + {Key: "name", Value: int32(1)}, + }}, + } + ctx := context.Background() + for _, c := range checks { + cursor, err := db.Collection(c.coll).Indexes().List(ctx) + require.NoError(t, err) + var idxes []bson.M + require.NoError(t, cursor.All(ctx, &idxes)) + found := false + for _, idx := range idxes { + keys, ok := idx["key"].(bson.M) + if !ok { + continue + } + if len(keys) != len(c.wantKeys) { + continue + } + match := true + for _, kv := range c.wantKeys { + if keys[kv.Key] != kv.Value { + match = false + break + } + } + if match { + found = true + break + } + } + assert.True(t, found, "expected index on %s with keys %v", c.coll, c.wantKeys) + } +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test-integration SERVICE=room-service` + +Expected: FAIL — the new indexes don't yet exist; one or more "expected index on …" assertions fail. + +- [ ] **Step 3: Implement** + +In `room-service/store_mongo.go`, edit `EnsureIndexes`. Insert the three new index creations near the existing apps index block (search for `assistant_name_idx`); append after the last existing CreateOne in the function but before `return nil`: + +```go + if _, err := s.apps.Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{ + {Key: "channelTab.default", Value: 1}, + {Key: "channelTab.enabled", Value: 1}, + {Key: "channelTab.name", Value: 1}, + }, + }); err != nil { + return fmt.Errorf("ensure apps (channelTab.default,enabled,name) index: %w", err) + } + if _, err := s.subscriptions.Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "roomId", Value: 1}, {Key: "u.isBot", Value: 1}}, + }); err != nil { + return fmt.Errorf("ensure subscriptions (roomId,u.isBot) index: %w", err) + } + if _, err := s.botCmdMenus.Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "activeStatus", Value: 1}, {Key: "name", Value: 1}}, + }); err != nil { + return fmt.Errorf("ensure bot_cmd_menu (activeStatus,name) index: %w", err) + } +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test-integration SERVICE=room-service` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/store_mongo.go room-service/integration_test.go +git commit -m "feat(room-service): index apps/subscriptions/bot_cmd_menu for new reads" +``` + +--- + +## Task 11: Add error sentinels and extend `sanitizeError` + +**Files:** +- Modify: `room-service/helper.go` + +- [ ] **Step 1: Add sentinels** + +In `room-service/helper.go`, locate the existing `var (...)` block that declares `errInvalidRole` etc. Append two new sentinels inside that block (keep the existing entries; show the last few lines for context): + +```go + errListLimitInvalid = errors.New("limit must be > 0") + errListOffsetInvalid = errors.New("offset must be >= 0") + + errAppAccessDenied = errors.New("not authorized to access this room's apps") + errResponseTooLarge = errors.New("response payload exceeds maximum size") +) +``` + +- [ ] **Step 2: Extend `sanitizeError`** + +Locate the `sanitizeError` function. Inside the second `case errors.Is(err, ...)` block (the long `case`/`errors.Is` chain), add the two new sentinels — e.g. immediately before `errors.Is(err, &dmExistsError{})`: + +```go + errors.Is(err, errAppAccessDenied), + errors.Is(err, errResponseTooLarge), + errors.Is(err, &dmExistsError{}), +``` + +- [ ] **Step 3: Confirm compiles** + +Run: `make test SERVICE=room-service` + +Expected: PASS (existing tests still green; the new sentinels are unused for now, which is fine — Go doesn't error on unused package-level vars). + +- [ ] **Step 4: Commit** + +```bash +git add room-service/helper.go +git commit -m "feat(room-service): add errAppAccessDenied and errResponseTooLarge sentinels" +``` + +--- + +## Task 12: Add `marshalBounded` + `replyBoundedJSON` helpers + +**Files:** +- Modify: `room-service/handler.go` (add `maxResponseBytes` field to `Handler`) +- Modify: `room-service/helper.go` (add helpers) +- Modify: `room-service/handler_test.go` (unit tests for `marshalBounded`) + +- [ ] **Step 1: Write failing tests** + +Append to `room-service/handler_test.go`: + +```go +func TestHandler_marshalBounded(t *testing.T) { + type sample struct { + Hello string `json:"hello"` + } + big := sample{Hello: strings.Repeat("x", 200)} + small := sample{Hello: "hi"} + + tests := []struct { + name string + maxResponseBytes int64 + value any + wantBodyEmpty bool + wantErrMsg string + }{ + {"under cap", 1024, small, false, ""}, + {"over cap", 64, big, true, errResponseTooLarge.Error()}, + {"disabled zero", 0, big, false, ""}, + {"disabled negative", -1, big, false, ""}, + {"marshal failure", 1024, func() {}, true, "internal error"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &Handler{maxResponseBytes: tt.maxResponseBytes} + body, errMsg := h.marshalBounded(tt.value) + assert.Equal(t, tt.wantErrMsg, errMsg) + if tt.wantBodyEmpty { + assert.Nil(t, body) + } else { + assert.NotEmpty(t, body) + } + }) + } +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=room-service` + +Expected: FAIL — `Handler` has no `maxResponseBytes` field; undefined `marshalBounded`. + +- [ ] **Step 3: Add `maxResponseBytes` field to `Handler`** + +In `room-service/handler.go`, edit the `Handler` struct definition (around line 31). Append a new field after `publishCore`: + +```go +type Handler struct { + store RoomStore + // keyStore is set when VALKEY_ADDRS is configured (always in production; tests may pass nil). + keyStore RoomKeyStore + memberListClient MemberListClient + msgReader MessageReader + siteID string + maxRoomSize int + maxBatchSize int + memberListTimeout time.Duration + publishToStream func(ctx context.Context, subj string, data []byte) error + publishCore func(ctx context.Context, subj string, data []byte) error + siteURL *url.URL + maxResponseBytes int64 +} +``` + +Add the `net/url` import at the top of the file if not already present: + +```go +import ( + // existing imports... + "net/url" + // rest... +) +``` + +(Do NOT yet wire `siteURL` / `maxResponseBytes` into `NewHandler` — that happens in Task 14.) + +- [ ] **Step 4: Add helpers to `helper.go`** + +Append to `room-service/helper.go`: + +```go +// marshalBounded marshals v and enforces h.maxResponseBytes (a value +// <= 0 disables the bound). On success returns (body, ""); on marshal +// failure returns (nil, "internal error"); on size violation returns +// (nil, errResponseTooLarge.Error()). +func (h *Handler) marshalBounded(v any) ([]byte, string) { + body, err := json.Marshal(v) + if err != nil { + slog.Error("marshal response failed", "error", err) + return nil, "internal error" + } + if h.maxResponseBytes > 0 && int64(len(body)) > h.maxResponseBytes { + slog.Error("response exceeds max payload", + "size", len(body), "limit", h.maxResponseBytes) + return nil, errResponseTooLarge.Error() + } + return body, "" +} + +// replyBoundedJSON wraps marshalBounded with the NATS send. nats* +// handlers use this in place of natsutil.ReplyJSON when a response +// payload could exceed the negotiated NATS max_payload. +func (h *Handler) replyBoundedJSON(msg *nats.Msg, v any) { + body, errMsg := h.marshalBounded(v) + if errMsg != "" { + natsutil.ReplyError(msg, errMsg) + return + } + if err := msg.Respond(body); err != nil { + slog.Error("reply failed", "error", err) + } +} +``` + +Add the missing imports at the top of `helper.go`: + +```go +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "regexp" + "strings" + + "github.com/nats-io/nats.go" + + "github.com/hmchangw/chat/pkg/model" + "github.com/hmchangw/chat/pkg/natsutil" +) +``` + +- [ ] **Step 5: Run — expect PASS** + +Run: `make test SERVICE=room-service` + +Expected: PASS for `TestHandler_marshalBounded` (all 5 subtests). Existing tests still green. + +- [ ] **Step 6: Commit** + +```bash +git add room-service/helper.go room-service/handler.go room-service/handler_test.go +git commit -m "feat(room-service): add marshalBounded and replyBoundedJSON helpers" +``` + +--- + +## Task 13: Add `authorizeRoomAppRead` helper + +**Files:** +- Modify: `room-service/handler.go` +- Modify: `room-service/handler_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `room-service/handler_test.go`: + +```go +func TestHandler_authorizeRoomAppRead(t *testing.T) { + tests := []struct { + name string + setupMock func(*MockRoomStore) + wantErr error + }{ + { + name: "member allowed", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{ + User: model.SubscriptionUser{ID: "u1", Account: "alice"}, + RoomID: "r1", + }, nil) + }, + wantErr: nil, + }, + { + name: "admin allowed (no sub, admin role)", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []string{"admin"}}, nil) + }, + wantErr: nil, + }, + { + name: "denied: no sub, no admin role", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []string{"user"}}, nil) + }, + wantErr: errAppAccessDenied, + }, + { + name: "denied: no sub, user not found (cross-site admin path)", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, ErrUserNotFound) + }, + wantErr: errAppAccessDenied, + }, + { + name: "transient sub-check error propagates", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, errors.New("mongo unavailable")) + }, + wantErr: nil, // checked separately below + }, + { + name: "transient user-lookup error propagates", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, errors.New("mongo unavailable")) + }, + wantErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + tt.setupMock(store) + h := &Handler{store: store, siteID: "site-a"} + err := h.authorizeRoomAppRead(context.Background(), "alice", "r1") + switch tt.name { + case "transient sub-check error propagates": + require.Error(t, err) + assert.Contains(t, err.Error(), "check room membership") + case "transient user-lookup error propagates": + require.Error(t, err) + assert.Contains(t, err.Error(), "check platform admin") + default: + if tt.wantErr == nil { + assert.NoError(t, err) + } else { + assert.ErrorIs(t, err, tt.wantErr) + } + } + }) + } +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=room-service` + +Expected: FAIL — undefined method `authorizeRoomAppRead` on `*Handler`. + +- [ ] **Step 3: Implement** + +Append to `room-service/handler.go` (any sensible spot — e.g. just before `natsCreateRoom`): + +```go +// authorizeRoomAppRead allows the request iff the caller has a +// subscription in roomID OR is a platform admin in the local users +// collection. Cross-site admin authority is out of scope: an admin +// whose users document lives on a different site is denied. +func (h *Handler) authorizeRoomAppRead(ctx context.Context, account, roomID string) error { + sub, err := h.store.GetSubscription(ctx, account, roomID) + if err != nil && !errors.Is(err, model.ErrSubscriptionNotFound) { + return fmt.Errorf("check room membership: %w", err) + } + if model.IsRoomMember(sub) { + return nil + } + user, err := h.store.GetUser(ctx, account) + if err != nil && !errors.Is(err, ErrUserNotFound) { + return fmt.Errorf("check platform admin: %w", err) + } + if model.IsPlatformAdmin(user) { + return nil + } + return errAppAccessDenied +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=room-service` + +Expected: PASS (all 6 subtests). + +- [ ] **Step 5: Commit** + +```bash +git add room-service/handler.go room-service/handler_test.go +git commit -m "feat(room-service): add authorizeRoomAppRead shared auth helper" +``` + +--- + +## Task 14: Wire `siteURL` + `nc.MaxPayload()` into the service + +**Files:** +- Modify: `room-service/main.go` +- Modify: `room-service/handler.go` + +- [ ] **Step 1: Extend `Config` and add validation in `main.go`** + +Edit `room-service/main.go`. Add `SiteURL` to the `config` struct (around line 22): + +```go +type config struct { + NatsURL string `env:"NATS_URL,required"` + NatsCredsFile string `env:"NATS_CREDS_FILE" envDefault:""` + SiteID string `env:"SITE_ID" envDefault:"site-local"` + SiteURL string `env:"SITE_URL,required"` + // ...all existing fields... +} +``` + +Add the `net/url` import at the top of `main.go`. + +Then, immediately after the `if cfg.MemberListTimeout <= 0` guard, insert SITE_URL validation: + +```go + siteURL, err := url.Parse(cfg.SiteURL) + if err != nil || siteURL.Scheme == "" || siteURL.Host == "" { + slog.Error("invalid SITE_URL: must be an absolute URL with scheme and host", + "value", cfg.SiteURL, "error", err) + os.Exit(1) + } +``` + +- [ ] **Step 2: Update `NewHandler` signature** + +Edit `room-service/handler.go`. Replace `NewHandler` (around line 45) with the extended signature: + +```go +func NewHandler( + store RoomStore, + keyStore RoomKeyStore, + memberListClient MemberListClient, + msgReader MessageReader, + siteID string, + maxRoomSize, maxBatchSize int, + memberListTimeout time.Duration, + publishToStream func(context.Context, string, []byte) error, + publishCore func(context.Context, string, []byte) error, + siteURL *url.URL, + maxResponseBytes int64, +) *Handler { + return &Handler{ + store: store, + keyStore: keyStore, + memberListClient: memberListClient, + msgReader: msgReader, + siteID: siteID, + maxRoomSize: maxRoomSize, + maxBatchSize: maxBatchSize, + memberListTimeout: memberListTimeout, + publishToStream: publishToStream, + publishCore: publishCore, + siteURL: siteURL, + maxResponseBytes: maxResponseBytes, + } +} +``` + +- [ ] **Step 3: Update the `NewHandler` call in `main.go`** + +Find the existing `handler := NewHandler(...)` call (around line 122). Append two new arguments — `siteURL` and `nc.MaxPayload()` — at the end: + +```go + handler := NewHandler(store, keyStore, memberListClient, cassReader, cfg.SiteID, cfg.MaxRoomSize, cfg.MaxBatchSize, cfg.MemberListTimeout, + func(ctx context.Context, subj string, data []byte) error { + if _, err := js.PublishMsg(ctx, natsutil.NewMsg(ctx, subj, data)); err != nil { + return fmt.Errorf("publish to %q: %w", subj, err) + } + return nil + }, + func(ctx context.Context, subj string, data []byte) error { + if err := nc.PublishMsg(ctx, natsutil.NewMsg(ctx, subj, data)); err != nil { + return fmt.Errorf("publish core to %q: %w", subj, err) + } + return nil + }, + siteURL, + nc.NatsConn().MaxPayload(), + ) +``` + +Note: `nc` is `*otelnats.Conn`; the underlying NATS connection is accessed via `nc.NatsConn()`. `MaxPayload()` is a method on `*nats.Conn`. + +- [ ] **Step 4: Build and confirm compiles** + +Run: `make test SERVICE=room-service` + +Expected: PASS. (Existing handler tests use `&Handler{...}` struct literals and don't go through `NewHandler`, so they're unaffected.) + +- [ ] **Step 5: Commit** + +```bash +git add room-service/main.go room-service/handler.go +git commit -m "feat(room-service): wire SITE_URL and NATS max-payload into the handler" +``` + +--- + +## Task 15: Implement `handleGetRoomAppTabs` (with URL rewrite) + +**Files:** +- Modify: `room-service/handler.go` +- Modify: `room-service/handler_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `room-service/handler_test.go`: + +```go +func newTabsTestHandler(t *testing.T, siteURL string) (*Handler, *MockRoomStore, *gomock.Controller) { + t.Helper() + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + u, err := url.Parse(siteURL) + require.NoError(t, err) + return &Handler{store: store, siteID: "site-a", siteURL: u}, store, ctrl +} + +func mockTabApp(id, tabName, urlTemplate string) model.App { + return model.App{ + ID: id, + AvatarURL: "https://cdn/" + id + ".png", + Assistant: &model.AppAssistant{Enabled: true, Name: id + ".bot"}, + ChannelTab: &model.AppChannelTab{ + Enabled: true, Default: true, Name: tabName, + URL: model.AppChannelTabURL{Default: urlTemplate}, + }, + } +} + +func TestHandler_handleGetRoomAppTabs_MemberAllowed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{ID: "u1", Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "Calendar", "https://upstream/cal/${roomId}/${siteId}/index"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.Apps, 1) + assert.Equal(t, "app1", resp.Apps[0].ID) + assert.Equal(t, "Calendar", resp.Apps[0].Name) + assert.Equal(t, "https://chat.example.com/cal/r1/site-a/index", resp.Apps[0].TabURL) + assert.Equal(t, "https://cdn/app1.png", resp.Apps[0].AvatarURL) + require.NotNil(t, resp.Apps[0].Assistant) + assert.Equal(t, "app1.bot", resp.Apps[0].Assistant.Name) +} + +func TestHandler_handleGetRoomAppTabs_AdminAllowed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []string{"admin"}}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{}, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Empty(t, resp.Apps) +} + +func TestHandler_handleGetRoomAppTabs_Denied(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []string{"user"}}, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppTabs_DeniedNoUser(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, ErrUserNotFound) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppTabs_EmptyResultIsEmptyArray(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return(nil, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotNil(t, resp.Apps, "must initialize empty slice, not nil, so JSON marshals to []") + assert.Len(t, resp.Apps, 0) + data, err := json.Marshal(resp) + require.NoError(t, err) + assert.Contains(t, string(data), `"apps":[]`) +} + +func TestHandler_handleGetRoomAppTabs_URLRewritePathPrefix(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com/chat") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "Calendar", "https://upstream/tab/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Equal(t, "https://chat.example.com/chat/tab/r1", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewriteStripsUserinfo(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "X", "https://user:pass@upstream/path/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotContains(t, resp.Apps[0].TabURL, "user") + assert.NotContains(t, resp.Apps[0].TabURL, "pass") + assert.Equal(t, "https://chat.example.com/path/r1", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewritePreservesQueryAndFragment(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "X", "https://upstream/path?room=${roomId}#tab=${siteId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Equal(t, "https://chat.example.com/path?room=r1#tab=site-a", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewriteSkipsEmptyAndMalformed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("ok1", "OK1", "https://upstream/ok1/${roomId}"), + mockTabApp("empty", "Empty", ""), + mockTabApp("bad", "Bad", "://malformed"), + mockTabApp("ok2", "OK2", "https://upstream/ok2/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.Apps, 2, "empty and malformed must be skipped") + assert.Equal(t, "ok1", resp.Apps[0].ID) + assert.Equal(t, "ok2", resp.Apps[1].ID) +} + +func TestHandler_handleGetRoomAppTabs_InvalidSubject(t *testing.T) { + h, _, _ := newTabsTestHandler(t, "https://chat.example.com") + _, err := h.handleGetRoomAppTabs(context.Background(), "not.a.valid.subject", nil) + assert.Error(t, err) +} + +func TestHandler_handleGetRoomAppTabs_StoreListError(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()). + Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppTabs_ContextTimeout(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + DoAndReturn(func(ctx context.Context, _, _ string) (*model.Subscription, error) { + <-ctx.Done() + return nil, ctx.Err() + }) + + parent, cancel := context.WithCancel(context.Background()) + cancel() + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(parent, subj, nil) + require.Error(t, err) + assert.True(t, + errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded), + "expected wrapped context error, got %v", err) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=room-service` + +Expected: FAIL — undefined `h.handleGetRoomAppTabs`. + +- [ ] **Step 3: Implement `buildTabURL` and `handleGetRoomAppTabs`** + +Append to `room-service/handler.go`: + +```go +// buildTabURL applies the SITE_URL-based scheme/host/path-prefix +// rewrite and the ${roomId}/${siteId} substitution to a channelTab URL +// template. Returns (url, true) on success; (_, false) when the +// template is empty or unparseable (caller should skip + warn). +func (h *Handler) buildTabURL(tmpl, roomID, siteID string) (string, bool) { + if tmpl == "" { + return "", false + } + // Substitute BEFORE parsing so url.URL.String() doesn't percent-encode + // the substituted values (roomID/siteID are URL-safe by construction). + tmpl = strings.ReplaceAll(tmpl, "${roomId}", roomID) + tmpl = strings.ReplaceAll(tmpl, "${siteId}", siteID) + u, err := url.Parse(tmpl) + if err != nil { + return "", false + } + joined := h.siteURL.JoinPath(u.Path) + joined.User = nil + joined.RawQuery = u.RawQuery + joined.Fragment = u.Fragment + return joined.String(), true +} + +// handleGetRoomAppTabs is the business logic for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs RPC. +func (h *Handler) handleGetRoomAppTabs(ctx context.Context, subj string, _ []byte) (model.GetRoomAppTabsResponse, error) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + account, roomID, ok := subject.ParseUserRoomSubject(subj) + if !ok { + return model.GetRoomAppTabsResponse{}, fmt.Errorf("invalid app-tabs subject: %s", subj) + } + + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes( + attribute.String("room.id", roomID), + attribute.String("site.id", h.siteID), + attribute.String("account", account), + ) + } + + if err := h.authorizeRoomAppRead(ctx, account, roomID); err != nil { + return model.GetRoomAppTabsResponse{}, err + } + + apps, err := h.store.ListDefaultChannelTabApps(ctx) + if err != nil { + return model.GetRoomAppTabsResponse{}, fmt.Errorf("list default channel-tab apps: %w", err) + } + + out := make([]model.RoomApp, 0, len(apps)) + for i := range apps { + app := &apps[i] + if app.ChannelTab == nil { + slog.Warn("skipping app with nil ChannelTab", "appId", app.ID, "roomId", roomID) + continue + } + tabURL, ok := h.buildTabURL(app.ChannelTab.URL.Default, roomID, h.siteID) + if !ok { + slog.Warn("skipping app with empty or unparseable channelTab url", + "appId", app.ID, "roomId", roomID, "template", app.ChannelTab.URL.Default) + continue + } + out = append(out, model.RoomApp{ + ID: app.ID, + Name: app.ChannelTab.Name, + TabURL: tabURL, + Assistant: app.Assistant, + AvatarURL: app.AvatarURL, + }) + } + return model.GetRoomAppTabsResponse{Apps: out}, nil +} + +// natsGetRoomAppTabs is the NATS-wrapping entry point for the app.tabs RPC. +func (h *Handler) natsGetRoomAppTabs(m otelnats.Msg) { + ctx := wrappedCtx(m) + resp, err := h.handleGetRoomAppTabs(ctx, m.Msg.Subject, m.Msg.Data) + if err != nil { + slog.Error("get room app tabs failed", "error", err, "subject", m.Msg.Subject) + natsutil.ReplyError(m.Msg, sanitizeError(err)) + return + } + h.replyBoundedJSON(m.Msg, resp) +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=room-service` + +Expected: PASS for all `TestHandler_handleGetRoomAppTabs_*` subtests. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/handler.go room-service/handler_test.go +git commit -m "feat(room-service): implement handleGetRoomAppTabs with URL rewrite" +``` + +--- + +## Task 16: Implement `handleGetRoomAppCommandMenu` + +**Files:** +- Modify: `room-service/handler.go` +- Modify: `room-service/handler_test.go` + +- [ ] **Step 1: Write failing tests** + +Append to `room-service/handler_test.go`: + +```go +func newCmdMenuTestHandler(t *testing.T) (*Handler, *MockRoomStore) { + t.Helper() + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + return &Handler{store: store, siteID: "site-a"}, store +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_NoBots(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{}, nil) + // ListActiveCmdMenus must NOT be called. + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotNil(t, resp.AppAssistants) + assert.Len(t, resp.AppAssistants, 0) + data, err := json.Marshal(resp) + require.NoError(t, err) + assert.Contains(t, string(data), `"appAssistants":[]`) +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_WithMenus(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "stocks.bot", AppName: "Stocks"}, + {AssistantName: "weather.bot", AppName: "Weather"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"stocks.bot", "weather.bot"}). + Return([]model.BotCmdMenu{ + {Name: "stocks.bot", CmdBlocks: []model.CmdBlock{{Text: "/quote"}}}, + {Name: "weather.bot", CmdBlocks: []model.CmdBlock{{Text: "/forecast"}}}, + }, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.AppAssistants, 2) + assert.Equal(t, "Stocks", resp.AppAssistants[0].AppName) + assert.Equal(t, "stocks.bot", resp.AppAssistants[0].Name) + require.Len(t, resp.AppAssistants[0].CmdBlocks, 1) + assert.Equal(t, "/quote", resp.AppAssistants[0].CmdBlocks[0].Text) + assert.Equal(t, "Weather", resp.AppAssistants[1].AppName) + assert.Equal(t, "/forecast", resp.AppAssistants[1].CmdBlocks[0].Text) +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_BotWithoutMenu(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "silent.bot", AppName: "Silent"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"silent.bot"}). + Return([]model.BotCmdMenu{}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.AppAssistants, 1) + assert.Equal(t, "Silent", resp.AppAssistants[0].AppName) + assert.Equal(t, "silent.bot", resp.AppAssistants[0].Name) + assert.Nil(t, resp.AppAssistants[0].CmdBlocks) +} + +func TestHandler_handleGetRoomAppCommandMenu_AdminAllowed(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []string{"admin"}}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + assert.Len(t, resp.AppAssistants, 0) +} + +func TestHandler_handleGetRoomAppCommandMenu_Denied(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice"}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppCommandMenu_InvalidSubject(t *testing.T) { + h, _ := newCmdMenuTestHandler(t) + _, err := h.handleGetRoomAppCommandMenu(context.Background(), "not.a.valid.subject", nil) + assert.Error(t, err) +} + +func TestHandler_handleGetRoomAppCommandMenu_StoreListRoomBotAppsError(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppCommandMenu_StoreListActiveCmdMenusError(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "weather.bot", AppName: "Weather"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"weather.bot"}). + Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppCommandMenu_ContextTimeout(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + DoAndReturn(func(ctx context.Context, _, _ string) (*model.Subscription, error) { + <-ctx.Done() + return nil, ctx.Err() + }) + + parent, cancel := context.WithCancel(context.Background()) + cancel() + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(parent, subj, nil) + require.Error(t, err) + assert.True(t, + errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded), + "expected wrapped context error, got %v", err) +} +``` + +- [ ] **Step 2: Run — expect FAIL** + +Run: `make test SERVICE=room-service` + +Expected: FAIL — undefined `h.handleGetRoomAppCommandMenu`. + +- [ ] **Step 3: Implement** + +Append to `room-service/handler.go`: + +```go +// handleGetRoomAppCommandMenu is the business logic for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu RPC. +func (h *Handler) handleGetRoomAppCommandMenu(ctx context.Context, subj string, _ []byte) (model.GetRoomAppCommandMenuResponse, error) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + account, roomID, ok := subject.ParseUserRoomSubject(subj) + if !ok { + return model.GetRoomAppCommandMenuResponse{}, fmt.Errorf("invalid app-cmd-menu subject: %s", subj) + } + + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes( + attribute.String("room.id", roomID), + attribute.String("site.id", h.siteID), + attribute.String("account", account), + ) + } + + if err := h.authorizeRoomAppRead(ctx, account, roomID); err != nil { + return model.GetRoomAppCommandMenuResponse{}, err + } + + bots, err := h.store.ListRoomBotApps(ctx, roomID) + if err != nil { + return model.GetRoomAppCommandMenuResponse{}, fmt.Errorf("list room bot apps: %w", err) + } + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes(attribute.Int("bot.count", len(bots))) + } + + if len(bots) == 0 { + return model.GetRoomAppCommandMenuResponse{ + AppAssistants: make([]model.RoomAppAssistant, 0), + }, nil + } + + names := make([]string, 0, len(bots)) + for _, b := range bots { + names = append(names, b.AssistantName) + } + menus, err := h.store.ListActiveCmdMenus(ctx, names) + if err != nil { + return model.GetRoomAppCommandMenuResponse{}, fmt.Errorf("list active cmd menus: %w", err) + } + byName := make(map[string][]model.CmdBlock, len(menus)) + for _, m := range menus { + byName[m.Name] = m.CmdBlocks + } + + out := make([]model.RoomAppAssistant, 0, len(bots)) + for _, b := range bots { + out = append(out, model.RoomAppAssistant{ + AppName: b.AppName, + Name: b.AssistantName, + CmdBlocks: byName[b.AssistantName], + }) + } + return model.GetRoomAppCommandMenuResponse{AppAssistants: out}, nil +} + +// natsGetRoomAppCommandMenu is the NATS-wrapping entry point. +func (h *Handler) natsGetRoomAppCommandMenu(m otelnats.Msg) { + ctx := wrappedCtx(m) + resp, err := h.handleGetRoomAppCommandMenu(ctx, m.Msg.Subject, m.Msg.Data) + if err != nil { + slog.Error("get room app cmd-menu failed", "error", err, "subject", m.Msg.Subject) + natsutil.ReplyError(m.Msg, sanitizeError(err)) + return + } + h.replyBoundedJSON(m.Msg, resp) +} +``` + +- [ ] **Step 4: Run — expect PASS** + +Run: `make test SERVICE=room-service` + +Expected: PASS for all `TestHandler_handleGetRoomAppCommandMenu_*` subtests. + +- [ ] **Step 5: Commit** + +```bash +git add room-service/handler.go room-service/handler_test.go +git commit -m "feat(room-service): implement handleGetRoomAppCommandMenu" +``` + +--- + +## Task 17: Wire both handlers into `RegisterCRUD` + +**Files:** +- Modify: `room-service/handler.go` + +- [ ] **Step 1: Extend `RegisterCRUD`** + +Edit `room-service/handler.go`. Locate `RegisterCRUD` (around line 66). Append two new subscriptions just before `return nil`: + +```go + if _, err := nc.QueueSubscribe(subject.RoomAppTabsWildcard(h.siteID), queue, h.natsGetRoomAppTabs); err != nil { + return fmt.Errorf("subscribe app tabs: %w", err) + } + if _, err := nc.QueueSubscribe(subject.RoomAppCmdMenuWildcard(h.siteID), queue, h.natsGetRoomAppCommandMenu); err != nil { + return fmt.Errorf("subscribe app cmd-menu: %w", err) + } + return nil +} +``` + +- [ ] **Step 2: Confirm compiles + tests pass** + +Run: `make test SERVICE=room-service` + +Expected: PASS. + +Run: `make lint` + +Expected: clean (no new lint errors). + +- [ ] **Step 3: Commit** + +```bash +git add room-service/handler.go +git commit -m "feat(room-service): register app.tabs and app.cmd-menu handlers" +``` + +--- + +## Task 18: Update `docs/client-api.md` and `docker-compose.yml` + +**Files:** +- Modify: `docs/client-api.md` +- Modify: `room-service/deploy/docker-compose.yml` + +- [ ] **Step 1: Inspect current docs for the format to mirror** + +Read `docs/client-api.md` and find the `#### List Members` entry under `### 3.1 room-service`. The new entries below follow the same shape (Subject, Request body, Success response, Error response, Triggered events). + +- [ ] **Step 2: Append the two new entries under §3.1 (room-service)** + +Add the following sections after the last existing `### 3.1` entry (e.g. after `#### Mute Toggle`), or insert in alphabetical order if the section enforces one: + +```markdown +#### Get Room App Tabs + +**Subject:** `chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs` +**Reply subject:** auto-generated `_INBOX.>` (NATS request/reply) + +- `{siteID}` must be the room's **origin `siteID`** (the site that owns the room), not the caller's own site. + +##### Request body + +Empty body (`{}` is tolerated). All inputs come from the subject. + +##### Success response + +| Field | Type | Notes | +|----------|----------------|-------| +| `apps` | array | Apps whose `channelTab.enabled` AND `channelTab.default` are both true, sorted by `channelTab.name asc`. Empty by default in DM/botDM rooms. | + +`RoomApp` fields: + +| Field | Type | Notes | +|-------------|---------------------|-------| +| `id` | string | `apps._id`. | +| `name` | string | `apps.channelTab.name`. | +| `tabUrl` | string | Computed: `SITE_URL`'s scheme/host/path-prefix + `apps.channelTab.url.default`'s path; `${roomId}` and `${siteId}` are substituted. Apps whose template URL is empty or unparseable are silently skipped. | +| `assistant` | object (optional) | `apps.assistant` subdocument if set. | +| `avatarUrl` | string (optional) | `apps.avatarUrl` if set. | + +##### Error response + +See [Error envelope](#6-error-envelope-reference). Common errors: `"not authorized to access this room's apps"` (caller is neither a room member nor a platform admin on the room's site), `"response payload exceeds maximum size"` (rare: response would exceed the NATS server's `max_payload`). + +##### Triggered events — success path + +`None — reply only.` + +##### Triggered events — error path + +`None — error returned only via the reply subject.` + +--- + +#### Get Room App Command Menu + +**Subject:** `chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu` +**Reply subject:** auto-generated `_INBOX.>` (NATS request/reply) + +- `{siteID}` must be the room's origin `siteID`. + +##### Request body + +Empty body (`{}` is tolerated). + +##### Success response + +| Field | Type | Notes | +|-----------------|-------------------------|-------| +| `appAssistants` | array | One entry per bot currently subscribed in the room whose owning app has `assistant.enabled=true`. Sorted by `name asc`. | + +`RoomAppAssistant` fields: + +| Field | Type | Notes | +|-------------|------------------|-------| +| `appName` | string | `apps.name`. | +| `name` | string | `apps.assistant.name` (the bot account). | +| `cmdBlocks` | array (optional) | Active command-menu blocks from `bot_cmd_menu` joined by name. Omitted/nil if no active menu exists for the bot. `CmdBlock` is recursive (`blocks` may contain further `CmdBlock`s) and may carry a `modal` object with `command` and `param`. | + +##### Error response + +Same envelope and sentinels as Get Room App Tabs. + +##### Triggered events — success path + +`None — reply only.` + +##### Triggered events — error path + +`None — error returned only via the reply subject.` +``` + +- [ ] **Step 3: Add `SITE_URL` to `room-service/deploy/docker-compose.yml`** + +Inspect the file first to see how env vars are declared. Add `SITE_URL=http://localhost:3000` to the `environment:` block of the `room-service` service. Example (only the env section is shown for context; preserve all other entries): + +```yaml + environment: + NATS_URL: nats://nats:4222 + MONGO_URI: mongodb://mongo:27017 + SITE_ID: site-local + SITE_URL: http://localhost:3000 + # ...other existing entries... +``` + +- [ ] **Step 4: Verify changes** + +Run: `make lint` + +Expected: clean. + +Run: `make test SERVICE=room-service` + +Expected: PASS. + +Read back the two new docs entries to confirm formatting is consistent with neighbours. + +- [ ] **Step 5: Commit** + +```bash +git add docs/client-api.md room-service/deploy/docker-compose.yml +git commit -m "docs(client-api): document app.tabs and app.cmd-menu RPCs + SITE_URL" +``` + +--- + +## Task 19: Final verification + +**Files:** (none — verification only) + +- [ ] **Step 1: Run the full unit-test suite** + +Run: `make test` + +Expected: ALL PASS, including the regenerated mocks and all new tests. + +- [ ] **Step 2: Run the full integration-test suite** + +Run: `make test-integration SERVICE=room-service` + +Expected: ALL PASS for the new `TestMongoStore_ListDefaultChannelTabApps`, `_ListRoomBotApps`, `_ListActiveCmdMenus`, `_EnsureIndexes_NewCompoundIndexes`, plus all existing room-service integration tests. + +- [ ] **Step 3: Run lint and SAST** + +Run: `make lint` + +Expected: clean. + +Run: `make sast` + +Expected: clean (no medium+ findings introduced). + +- [ ] **Step 4: Confirm coverage** + +Run: `go test -coverprofile=/tmp/room-service.cover.out -race ./room-service/... && go tool cover -func=/tmp/room-service.cover.out | tail -20` + +Expected: package-level coverage ≥80%; new handler methods and store methods should each report ≥90%. If `handleGetRoomAppTabs`, `handleGetRoomAppCommandMenu`, `marshalBounded`, `authorizeRoomAppRead`, `buildTabURL`, `ListDefaultChannelTabApps`, `ListRoomBotApps`, or `ListActiveCmdMenus` are below 90%, add focused tests for the uncovered branch before merging. + +- [ ] **Step 5: Push the branch** + +Run: `git push -u origin claude/determined-fermi-A07HN` + +Expected: success (force-with-lease not needed unless the rebase changed history). + +--- + +## Coverage map vs spec + +| Spec section | Implemented in | +|---|---| +| Subjects + Subject Builders | Task 0 | +| Wire Format | Tasks 1, 3, 4, 5 | +| Model additions (User.Roles, IsPlatformAdmin, IsRoomMember, App.AvatarURL/ChannelTab, BotCmdMenu/CmdBlock/CmdModal) | Tasks 1, 2, 3, 4, 5 | +| Authorization helper | Task 13 | +| Handler flow (timeout + OTel + tabs + cmd-menu + nats wrappers + RegisterCRUD) | Tasks 14, 15, 16, 17 | +| Errors (sentinels + sanitizeError) | Task 11 | +| Response Payload Cap (marshalBounded + replyBoundedJSON) | Task 12 | +| Stores (RoomBotAppEntry, three methods, EnsureIndexes additions, botCmdMenus collection) | Tasks 6, 7, 8, 9, 10 | +| URL Rewrite | Task 15 (`buildTabURL`) | +| Wiring (SITE_URL, NewHandler signature, nc.MaxPayload) | Task 14 | +| Local dev (SITE_URL in docker-compose) | Task 18 | +| Client API Doc | Task 18 | +| Testing (subject, model, handler unit, response cap, integration) | Tasks 0, 1–5, 12, 13, 15, 16; integration in 7, 8, 9, 10 | +| Future optimizations | Spec-only (deferred) | +| Risks | Spec-only | diff --git a/docs/superpowers/specs/2026-05-26-room-service-app-tabs-and-cmd-menu-design.md b/docs/superpowers/specs/2026-05-26-room-service-app-tabs-and-cmd-menu-design.md new file mode 100644 index 000000000..b6747d769 --- /dev/null +++ b/docs/superpowers/specs/2026-05-26-room-service-app-tabs-and-cmd-menu-design.md @@ -0,0 +1,677 @@ +# Room App Tabs & Command Menu RPCs (room-service) + +## Summary + +Add two read-only, client-facing NATS request/reply RPCs to `room-service` +that expose per-room app metadata: + +- `GetRoomAppTabs` — returns the apps with default channel tabs, each with + a per-room tab URL produced by substituting `${roomId}` and `${siteId}` + into the app's URL template and rewriting scheme+host+path-prefix to + the local `SITE_URL`. +- `GetRoomAppCommandMenu` — for every bot subscribed to the room, returns + the bot's active command-menu blocks joined with the owning app's name. + +Both handlers are gated by the same rule: caller must either be a +member of the room (a `subscriptions` row exists for `(account, roomID)`) +or a platform admin (caller's `users` doc carries `"admin"` in +`Roles`). Admin check is **per-site** — room-service consults only +its local `users` collection (the room's site). An admin whose +`users` doc lives on another site is denied. All data comes from +local Mongo (`subscriptions`, `users`, `apps`, `bot_cmd_menu`); +no external lookups. + +## Subjects + +| Method | Concrete | Wildcard | +|---|---|---| +| `GetRoomAppTabs` | `chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs` | `chat.user.*.request.room.*.{siteID}.app.tabs` | +| `GetRoomAppCommandMenu` | `chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu` | `chat.user.*.request.room.*.{siteID}.app.cmd-menu` | + +Queue group: `room-service` (mirrors every other `room-service` handler). +Subject parsing reuses `subject.ParseUserRoomSubject`, which walks for +the `room` token and works for any room-scoped form regardless of +trailing-token count (the new subjects are 9-token; existing +`message.read`-style subjects are 7-token). + +## Wire Format + +Both handlers accept an empty body (`{}` is tolerated). Every input is +encoded in the subject; there are no request bodies to validate. + +`pkg/model/app.go` (additions): + +```go +type RoomApp struct { + ID string `json:"id"` + Name string `json:"name"` // = apps.channelTab.name + TabURL string `json:"tabUrl"` // computed (see "URL Rewrite") + Assistant *AppAssistant `json:"assistant,omitempty"` + AvatarURL string `json:"avatarUrl,omitempty"` +} + +type GetRoomAppTabsResponse struct { + Apps []RoomApp `json:"apps"` +} + +type RoomAppAssistant struct { + AppName string `json:"appName"` // = apps.name + Name string `json:"name"` // = apps.assistant.name (bot account) + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty"` +} + +type GetRoomAppCommandMenuResponse struct { + AppAssistants []RoomAppAssistant `json:"appAssistants"` +} +``` + +Empty result: `{"apps":[]}` / `{"appAssistants":[]}` — always 200 OK, never +an error. + +## Subject Builders + +`pkg/subject/subject.go` (additions): + +```go +func RoomAppTabs(account, roomID, siteID string) string { + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.tabs", account, roomID, siteID) +} + +func RoomAppTabsWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.tabs", siteID) +} + +func RoomAppCmdMenu(account, roomID, siteID string) string { + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.cmd-menu", account, roomID, siteID) +} + +func RoomAppCmdMenuWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.cmd-menu", siteID) +} +``` + +## Model additions + +`pkg/model/user.go`: + +```go +// Roles carries platform-level role assignments (e.g. "user", "admin"). +// Absent on legacy user documents — treat nil as "no platform roles". +Roles []string `json:"roles,omitempty" bson:"roles,omitempty"` + +// PlatformRoleAdmin is the platform-admin role string carried in User.Roles. +const PlatformRoleAdmin = "admin" + +// IsPlatformAdmin reports whether u holds the platform admin role. +// Returns false for nil receivers and for users without the role. +func IsPlatformAdmin(u *User) bool { + if u == nil { + return false + } + for _, r := range u.Roles { + if r == PlatformRoleAdmin { + return true + } + } + return false +} +``` + +`pkg/model/subscription.go`: + +```go +// IsRoomMember reports whether sub represents an active membership. +// Returns false for nil so callers can pass the result of a store lookup +// that returned (nil, ErrSubscriptionNotFound) — the caller is expected +// to have already classified the error and set sub to nil on not-found. +func IsRoomMember(sub *Subscription) bool { + return sub != nil +} +``` + +`pkg/model/app.go`: + +```go +type App struct { + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + AvatarURL string `json:"avatarUrl,omitempty" bson:"avatarUrl,omitempty"` // NEW + Assistant *AppAssistant `json:"assistant,omitempty" bson:"assistant,omitempty"` + ChannelTab *AppChannelTab `json:"channelTab,omitempty" bson:"channelTab,omitempty"` // NEW + Sponsors []AppSponsor `json:"sponsors,omitempty" bson:"sponsors,omitempty"` +} + +type AppChannelTab struct { + Enabled bool `json:"enabled" bson:"enabled"` + Default bool `json:"default" bson:"default"` + Name string `json:"name" bson:"name"` + URL AppChannelTabURL `json:"url" bson:"url"` +} + +type AppChannelTabURL struct { + Default string `json:"default" bson:"default"` +} +``` + +`pkg/model/botcmdmenu.go` (new file): + +```go +package model + +// BotCmdMenu is a row in the bot_cmd_menu collection. Name matches an +// AppAssistant.Name (the bot account) and joins back to the owning App via +// that field. ActiveStatus gates whether the menu is currently exposed. +type BotCmdMenu struct { + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + ActiveStatus bool `json:"activeStatus" bson:"activeStatus"` + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty" bson:"cmdBlocks,omitempty"` +} + +// CmdBlock is the recursive building block of a bot command menu. +type CmdBlock struct { + Text string `json:"text,omitempty" bson:"text,omitempty"` + ActionType string `json:"actionType,omitempty" bson:"actionType,omitempty"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + Payload string `json:"payload,omitempty" bson:"payload,omitempty"` + Modal *CmdModal `json:"modal,omitempty" bson:"modal,omitempty"` + Blocks []CmdBlock `json:"blocks,omitempty" bson:"blocks,omitempty"` +} + +// CmdModal carries the slash-style command + param a modal-triggering +// block invokes. CmdModal does NOT nest its own blocks — recursive +// rendering happens via CmdBlock.Blocks on the enclosing CmdBlock. +type CmdModal struct { + Command string `json:"command,omitempty" bson:"command,omitempty"` + Param string `json:"param,omitempty" bson:"param,omitempty"` +} +``` + +## Authorization helper + +`room-service/handler.go`: + +```go +func (h *Handler) authorizeRoomAppRead(ctx context.Context, account, roomID string) error { + sub, err := h.store.GetSubscription(ctx, account, roomID) + if err != nil && !errors.Is(err, model.ErrSubscriptionNotFound) { + return fmt.Errorf("check room membership: %w", err) + } + if model.IsRoomMember(sub) { + return nil + } + user, err := h.store.GetUser(ctx, account) + if err != nil && !errors.Is(err, ErrUserNotFound) { + return fmt.Errorf("check platform admin: %w", err) + } + if !model.IsPlatformAdmin(user) { + return errAppAccessDenied + } + // Admin bypass: verify the room exists, else fabricated room IDs + // would receive plausible-looking responses. + if _, err := h.store.GetRoom(ctx, roomID); err != nil { + if errors.Is(err, mongo.ErrNoDocuments) { + return errAppAccessDenied + } + return fmt.Errorf("check room existence: %w", err) + } + return nil +} +``` + +Used unchanged by both handlers. Subscription check first (common +case); admin fallback second AND requires the room to exist. Local-site +Mongo only — see Summary on cross-site admin scope. + +## Handler flow + +Both handlers start with `ctx, cancel := context.WithTimeout(ctx, 5*time.Second); defer cancel()` +(mirrors `handleRoomsInfoBatch`) and set OTel attributes +`room.id`, `site.id`, `account` on the active span (mirrors +`handleCreateRoom`). `handleGetRoomAppCommandMenu` also records +`bot.count` after step 3. `siteID` for URL substitution and tracing +comes from `h.siteID` (the wildcard subscription binds the subject's +`{siteID}` to the service's own site). + +`handleGetRoomAppTabs`: + +1. `subject.ParseUserRoomSubject(subj)` → `(account, roomID)`. +2. `authorizeRoomAppRead(ctx, account, roomID)`. +3. `apps, err := h.store.ListDefaultChannelTabApps(ctx)`. +4. Initialize `out := make([]model.RoomApp, 0, len(apps))` so an empty + result marshals to `[]` not `null`. +5. For each `app`, build `tabUrl` via the URL Rewrite rules below; if the + rewrite fails (empty/unparseable URL), `slog.Warn` and skip the entry. +6. Reply `GetRoomAppTabsResponse{Apps: out}`. + +DM/botDM rooms return empty by construction (no `GetRoom` pre-filter). + +`handleGetRoomAppCommandMenu`: + +1. `subject.ParseUserRoomSubject(subj)` → `(account, roomID)`. +2. `authorizeRoomAppRead(ctx, account, roomID)`. +3. `bots, err := h.store.ListRoomBotApps(ctx, roomID)`. If empty, reply + `{"appAssistants":[]}` (with the slice initialized as + `make([]model.RoomAppAssistant, 0)`) without touching `bot_cmd_menu`. +4. Collect the distinct `assistantName` values; call + `h.store.ListActiveCmdMenus(ctx, assistantNames)`. +5. Build a `name → CmdBlocks` map from the returned menus + (`bot_cmd_menu` invariant: at most one active row per `name`; see + Stores). Initialize `out := make([]model.RoomAppAssistant, 0, len(bots))`; + for each `(appName, assistantName)` from step 3, emit a + `RoomAppAssistant` with `CmdBlocks` from the map (nil if absent). +6. Reply `GetRoomAppCommandMenuResponse{AppAssistants: out}`. + +`natsGetRoomAppTabs` / `natsGetRoomAppCommandMenu` follow the standard +`room-service` wrapper, with one difference: success replies go through +the `replyBoundedJSON` helper (see "Response Payload Cap") so an +oversized payload surfaces as a clean error envelope rather than a +NATS-level `ErrMaxPayload`: + +```go +func (h *Handler) natsGetRoomAppTabs(m otelnats.Msg) { + ctx := wrappedCtx(m) + resp, err := h.handleGetRoomAppTabs(ctx, m.Msg.Subject, m.Msg.Data) + if err != nil { + slog.Error("get room app tabs failed", "error", err, "subject", m.Msg.Subject) + natsutil.ReplyError(m.Msg, sanitizeError(err)) + return + } + h.replyBoundedJSON(m.Msg, resp) +} +``` + +Both register via `RegisterCRUD` with the standard queue group: + +```go +if _, err := nc.QueueSubscribe(subject.RoomAppTabsWildcard(h.siteID), queue, h.natsGetRoomAppTabs); err != nil { + return fmt.Errorf("subscribe app tabs: %w", err) +} +if _, err := nc.QueueSubscribe(subject.RoomAppCmdMenuWildcard(h.siteID), queue, h.natsGetRoomAppCommandMenu); err != nil { + return fmt.Errorf("subscribe app cmd-menu: %w", err) +} +``` + +## Errors + +> **Implementation note — rebased onto `main` after #250 "centralized error +> codes".** `room-service` no longer uses `sanitizeError` + `natsutil.ReplyError` +> (both removed by #250). The sentinels below are now typed `*errcode.Error` and +> replies go through `errnats.Reply`, which classifies + logs once: +> `errAppAccessDenied = errcode.Forbidden("not authorized to access this room's apps", errcode.WithReason(errcode.RoomNotMember))` +> and `errResponseTooLarge = errcode.Internal("response payload exceeds maximum size")`. +> Invalid-subject parse failures return `errcode.BadRequest("invalid request")`. +> The original `errors.New` + allow-list design below is kept for historical context. + +Add to `room-service/helper.go` (sentinels): + +```go +errAppAccessDenied = errors.New("not authorized to access this room's apps") +errResponseTooLarge = errors.New("response payload exceeds maximum size") +``` + +Add both to the `sanitizeError` switch's allow-list so the literal +sentinel message is surfaced verbatim to clients (no leakage of +internal context). No other new sentinels — the URL-rewrite skip is +silent (warn-only), and empty results return 200 OK with an empty +array. + +## Response Payload Cap + +> **Implementation note (post-#250).** `marshalBounded` now returns +> `([]byte, error)` — `errResponseTooLarge` (an `*errcode.Error`) on overflow, a +> wrapped marshal error otherwise — and `replyBoundedJSON(ctx, msg, v)` sends the +> error via `errnats.Reply`. The `([]byte, string)` + `natsutil.ReplyError` design +> below is historical. + +Replies larger than the negotiated `max_payload` (ops runs at 64KB) +would otherwise be dropped silently by NATS. The handler short-circuits +with `errResponseTooLarge` instead. Pure marshaling + bound check lives +in a testable helper: + +```go +// room-service/helper.go + +// marshalBounded marshals v and enforces h.maxResponseBytes (<= 0 +// disables the bound). Returns (body, "") or (nil, errMsg). +func (h *Handler) marshalBounded(v any) ([]byte, string) { + body, err := json.Marshal(v) + if err != nil { + slog.Error("marshal response failed", "error", err) + return nil, "internal error" + } + if h.maxResponseBytes > 0 && int64(len(body)) > h.maxResponseBytes { + slog.Error("response exceeds max payload", + "size", len(body), "limit", h.maxResponseBytes) + return nil, errResponseTooLarge.Error() + } + return body, "" +} + +func (h *Handler) replyBoundedJSON(msg *nats.Msg, v any) { + body, errMsg := h.marshalBounded(v) + if errMsg != "" { + natsutil.ReplyError(msg, errMsg) + return + } + if err := msg.Respond(body); err != nil { + slog.Error("reply failed", "error", err) + } +} +``` + +`maxResponseBytes` is sourced from `nc.MaxPayload()` after connect (in +`main.go`) and passed into `NewHandler`. Applied to both handlers so +`app.tabs` shares the same guard. + +## Stores + +`room-service/store.go`: + +```go +// RoomBotAppEntry pairs an assistant's bot account with its owning app +// name — the joined output of ListRoomBotApps. +type RoomBotAppEntry struct { + AssistantName string `bson:"assistantName"` + AppName string `bson:"appName"` +} + +// ListDefaultChannelTabApps returns apps whose channelTab.enabled AND +// channelTab.default are both true, sorted by channelTab.name asc. +// Projection: _id, avatarUrl, assistant, channelTab (app.name is +// excluded; the response uses channelTab.name). Empty result is +// ([], nil). +ListDefaultChannelTabApps(ctx context.Context) ([]model.App, error) + +// ListRoomBotApps runs a single aggregation against `subscriptions` that +// $matches { roomId, "u.isBot": true }, $lookups `apps` where +// assistant.enabled AND assistant.name == "$u.account", $unwinds the +// joined app, and projects { assistantName: "$app.assistant.name", +// appName: "$app.name" }. Empty result is ([], nil). +ListRoomBotApps(ctx context.Context, roomID string) ([]RoomBotAppEntry, error) + +// ListActiveCmdMenus returns bot_cmd_menu documents where activeStatus +// is true AND name IN assistantNames, sorted by name asc. Returns +// ([], nil) when assistantNames is empty (skips the query entirely). +ListActiveCmdMenus(ctx context.Context, assistantNames []string) ([]model.BotCmdMenu, error) +``` + +`room-service/store_mongo.go`: + +- `MongoStore` gains one new collection handle: `botCmdMenus = db.Collection("bot_cmd_menu")`. +- `ListDefaultChannelTabApps` — `Find` with filter + `{"channelTab.enabled": true, "channelTab.default": true}`, sort + `{"channelTab.name": 1}`, projection + `{_id:1, avatarUrl:1, assistant:1, channelTab:1}` (`name` excluded — + the response uses `channelTab.name`, not `app.name`). +- `ListRoomBotApps` — `Aggregate` on `subscriptions` with the pipeline. + The trailing `$sort` on `assistantName` makes the response order + deterministic for tests and clients: + ```json + [ {"$match": {"roomId": roomID, "u.isBot": true}}, + {"$lookup": { + "from": "apps", + "let": {"acct": "$u.account"}, + "pipeline": [ + {"$match": {"$expr": {"$and": [ + {"$eq": ["$assistant.enabled", true]}, + {"$eq": ["$assistant.name", "$$acct"]} ]}}}, + {"$project": {"_id": 0, + "assistantName": "$assistant.name", + "appName": "$name"}} ], + "as": "app"}}, + {"$unwind": "$app"}, + {"$replaceRoot": {"newRoot": "$app"}}, + {"$sort": {"assistantName": 1}} ] + ``` +- `ListActiveCmdMenus` — `Find` with filter + `{"activeStatus": true, "name": {"$in": assistantNames}}`, sort + `{"name": 1}`, projection `{_id:0, name:1, cmdBlocks:1}`. Upstream + invariant: at most one active row per `name` (recommend a partial + unique index `{name:1} partialFilterExpression:{activeStatus:true}` + on the writer side — out of scope here). On violation, the sort + makes last-write-wins deterministic. + +`EnsureIndexes` additions (best-effort, idempotent): + +- `apps`: `{channelTab.default: 1, channelTab.enabled: 1, channelTab.name: 1}` + (compound; serves `ListDefaultChannelTabApps` filter **and** sort + from a single index scan). +- `subscriptions`: `{roomId: 1, "u.isBot": 1}` (compound; serves the + `$match` stage of `ListRoomBotApps` directly, instead of riding the + existing `(roomId, u.account)` index on the `roomId` prefix and + filtering `u.isBot` in memory). +- `bot_cmd_menu`: `{activeStatus: 1, name: 1}` (compound, ESR order — + activeStatus is the equality match, name is the `$in` selector; + serves `ListActiveCmdMenus`). + +`assistant.name` already has an index in `EnsureIndexes` (added for +botDM creation) and is reused by the `$lookup` step in `ListRoomBotApps`. + +`mock_store_test.go` regenerated via `make generate SERVICE=room-service`. + +## URL Rewrite + +For each app returned by `ListDefaultChannelTabApps`: + +1. If `app.ChannelTab == nil || app.ChannelTab.URL.Default == ""`: skip + warn. +2. Substitute placeholders **before** parsing (otherwise `url.URL.String()` + percent-encodes the substituted values): + ```go + tmpl = strings.ReplaceAll(app.ChannelTab.URL.Default, "${roomId}", roomID) + tmpl = strings.ReplaceAll(tmpl, "${siteId}", h.siteID) + ``` + `roomID` / `siteID` are URL-safe by construction (subject parsing + rejects NATS wildcards; ID generators produce base62/UUIDv7-hex). +3. `u, err := url.Parse(tmpl)`. On error: skip + warn. +4. Merge with the configured `SITE_URL`, preserving its path prefix: + ```go + joined := h.siteURL.JoinPath(u.Path) + joined.User = nil // strip any userinfo + joined.RawQuery = u.RawQuery + joined.Fragment = u.Fragment + ``` + `JoinPath` carries `Scheme`/`Host` from `h.siteURL` and collapses + duplicate slashes. Template's `Scheme`/`Host` are discarded. +5. `tabUrl := joined.String()`. + +`h.siteURL` is parsed once at construction from `SITE_URL`; `main.go` +exits non-zero if `Scheme` or `Host` is empty. `siteURL.Path` (e.g. +`/chat`) is preserved as a prefix on every generated `tabUrl`. + +## Wiring + +`room-service/main.go`: + +```go +type config struct { + // ...existing fields... + SiteURL string `env:"SITE_URL,required"` +} +``` + +Startup validation (before constructing the handler): + +```go +siteURL, err := url.Parse(cfg.SiteURL) +if err != nil || siteURL.Scheme == "" || siteURL.Host == "" { + slog.Error("invalid SITE_URL: must be an absolute URL with scheme and host", + "value", cfg.SiteURL, "error", err) + os.Exit(1) +} +``` + +`siteURL.Path` is allowed and preserved (see URL Rewrite). Trailing +slashes are normalized by `JoinPath`. + +`Handler` gains two fields, threaded through `NewHandler`: + +```go +siteURL *url.URL // pre-parsed SITE_URL +maxResponseBytes int64 // = nc.MaxPayload(), set in main.go after natsutil.Connect +``` + +Tests exercising the new handlers MUST set `siteURL` (otherwise +`JoinPath` panics on nil); `maxResponseBytes` is optional and defaults +to 0 (cap disabled). + +## Local dev + +`room-service/deploy/docker-compose.yml`: add `SITE_URL=http://localhost:3000` +(or whatever absolute URL — including optional path prefix — the local +frontend runs behind) to the `room-service` service environment. + +No Dockerfile change. No CI change (env-only). + +## Client API Doc + +Per CLAUDE.md §5, update `docs/client-api.md` in the same PR. Add two +entries under §3.1 (room-service) following the existing `member.list` +format: subject, request body shape, success response shape, error +envelope, and `Triggered events` section ("None — reply only" for both). + +## Testing (TDD) + +### Subject builders (`pkg/subject/subject_test.go`) + +- `TestRoomAppTabs` / `TestRoomAppTabsWildcard` — exact-string asserts. +- `TestRoomAppCmdMenu` / `TestRoomAppCmdMenuWildcard` — exact-string asserts. +- `TestRoomAppTabs_ParseUserRoomSubject` — round-trip parse via the + shared parser. +- `TestRoomAppCmdMenu_ParseUserRoomSubject` — round-trip parse. + +### Model round-trips (`pkg/model/model_test.go`) + +- `TestUserJSON_WithRoles` + omitempty check (nil/empty Roles must NOT + appear in JSON output). +- `TestAppRoundtrip_WithChannelTabAndAvatar` — App with ChannelTab and + AvatarURL populated. +- `TestAppChannelTabRoundtrip` — standalone round-trip. +- `TestBotCmdMenuRoundtrip` — with active and inactive menus. +- `TestCmdBlockRoundtrip_Recursive` — block with `Modal` set, plus nested + `Blocks` on the enclosing block (recursion lives on `CmdBlock`, not + `CmdModal`). +- `TestGetRoomAppTabsResponseRoundtrip` and + `TestGetRoomAppCommandMenuResponseRoundtrip` — full wrapper structs. +- `TestIsPlatformAdmin` — nil receiver false; empty roles false; + "admin" present true; "admin" absent false. +- `TestIsRoomMember` — nil false; non-nil true. + +### Handler unit tests (`room-service/handler_test.go`) + +`TestHandler_handleGetRoomAppTabs` table-driven: + +| Case | Setup | Expected outcome | +|------|-------|------------------| +| member allowed | `GetSubscription` returns sub | 200, apps from store, URL rewrite applied | +| admin allowed | `GetSubscription` → `ErrSubscriptionNotFound`; `GetUser` returns user with `Roles=["admin"]` | 200, apps returned | +| denied | `GetSubscription` → not-found; `GetUser` returns user without admin role | `errAppAccessDenied` | +| denied (no user) | `GetSubscription` → not-found; `GetUser` → `ErrUserNotFound` | `errAppAccessDenied` (covers the cross-site-admin path: their `users` doc is absent on this site) | +| empty result | `ListDefaultChannelTabApps` returns `nil` | `{"apps":[]}` (verified by JSON-encoding the response and asserting on the literal `[]`, not `null`) | +| URL rewrite happy path | app with `https://upstream.example.com/app/${roomId}/${siteId}/index`, `SITE_URL=https://chat.example.com` | `https://chat.example.com/app///index` | +| URL rewrite preserves SITE_URL path prefix | `SITE_URL=https://chat.example.com/chat`, template path `/tab/${roomId}` | `https://chat.example.com/chat/tab/` | +| URL rewrite strips userinfo | template `https://user:pass@upstream/path/${roomId}` | `/path/` — no userinfo in result | +| URL rewrite preserves query and fragment | template `https://upstream/path?room=${roomId}#tab=${siteId}` | `/path?room=#tab=` | +| URL rewrite multiple apps sorted | store returns 3 apps in `channelTab.name asc` | order preserved, all URLs rewritten | +| URL rewrite empty url | mixed input including `ChannelTab.URL.Default == ""` | bad entry skipped, others emitted | +| URL rewrite malformed url | mixed input including `"://bad"` | bad entry skipped, others emitted | +| invalid subject | subj missing `room` token | parse error | +| sub-check transient error | `GetSubscription` returns a non-sentinel error | wrapped error | +| user-lookup transient error | `GetUser` returns a non-sentinel error | wrapped error | +| store list error | `ListDefaultChannelTabApps` errors | wrapped error | +| context timeout propagation | mock reads `ctx.Done()` and returns `ctx.Err()`; test passes a pre-cancelled parent | wrapped context error (`Canceled` or `DeadlineExceeded`) | + +`TestHandler_handleGetRoomAppCommandMenu` table-driven: + +| Case | Setup | Expected outcome | +|------|-------|------------------| +| member allowed, no bots | `ListRoomBotApps` returns `[]` | `{"appAssistants":[]}` (literal `[]`, not `null`), `ListActiveCmdMenus` NOT called | +| member allowed, bots with active menus | bots `[{a, A}, {b, B}]`, menus `[{name:a,blocks:[...]},{name:b,blocks:[...]}]` | both entries with their blocks; order matches the store's `assistantName asc` | +| member allowed, bot with no active menu | bots `[{a, A}]`, menus `[]` | one entry with nil cmdBlocks | +| admin allowed | sub not-found + admin user | same as member path | +| denied | sub not-found + non-admin user | `errAppAccessDenied` | +| invalid subject | malformed `subj` | parse error | +| store list error (bots) | `ListRoomBotApps` errors | wrapped error | +| store list error (menus) | `ListActiveCmdMenus` errors | wrapped error | +| context timeout propagation | mock reads `ctx.Done()`; pre-cancelled parent | wrapped context error | + +### Response cap (`room-service/handler_test.go`) + +`TestHandler_marshalBounded` table-driven, no NATS connection +required — exercises the pure helper: + +| Case | Setup | Expected outcome | +|------|-------|------------------| +| under cap | `Handler.maxResponseBytes = 1024`, value marshals to 512 bytes | `(body, "")` returned; `body` is the marshaled JSON | +| over cap | `Handler.maxResponseBytes = 64`, value marshals to ~200 bytes | `(nil, errResponseTooLarge.Error())` returned | +| disabled (zero) | `Handler.maxResponseBytes = 0` | `(body, "")` returned regardless of body size | +| marshal failure | value is a `func() {}` (json.Marshal errors on funcs) | `(nil, "internal error")` returned | + +Mocks: `make generate SERVICE=room-service` regenerates +`mock_store_test.go` with the three new methods. + +### Integration tests (`room-service/integration_test.go`, build tag `integration`) + +- `TestMongoStore_ListDefaultChannelTabApps` — seed 4 apps: two + `channelTab.default=true,enabled=true` (one of each name order), one + `enabled=false`, one with no `channelTab` field; assert exactly the + two default+enabled apps come back in `channelTab.name asc` order. +- `TestMongoStore_ListRoomBotApps` — seed users, apps (one with + `assistant.enabled=true` + matching `name`, one with + `assistant.enabled=false`, one with no assistant), subscriptions in + two rooms (one bot in roomA, one bot in roomB, one human in roomA); + assert `ListRoomBotApps("roomA")` returns only the bot whose app has + `assistant.enabled=true`. A second assertion attempts to insert a + second subscription for the same `(roomA, botAccount)` pair and + expects a `mongo.IsDuplicateKeyError` — exercises the + `(roomId, u.account)` unique-index invariant that protects + `ListRoomBotApps` from returning duplicate rows. +- `TestMongoStore_ListActiveCmdMenus` — seed three menu docs (one + `activeStatus=true` with matching name, one `activeStatus=false`, + one with non-matching name); assert only the first comes back. +- `TestMongoStore_ListActiveCmdMenus_Empty` — empty `assistantNames` + returns `([], nil)` without hitting Mongo. +- `TestMongoStore_EnsureIndexes_NewCompoundIndexes` — after + `EnsureIndexes`, assert the new compound indexes on `apps`, + `subscriptions`, and `bot_cmd_menu` exist with the expected key + ordering (cheap regression guard against accidental removal). + +### Coverage + +≥80% package-wide per CLAUDE.md mandate. Aim ≥90% on the new handler +methods and the three new store methods. + +## Out of scope + +- Cross-site federation (no outbox events, no inbox handler changes, + no cross-site admin authority — see Summary). +- Write paths (no new MongoDB writes; this PR is read-only). +- JetStream streams, canonical events, room-worker involvement. +- Notifications, push, or pub/sub event emission. +- Frontend changes — owned by `chat-frontend` in a separate PR. +- Backfilling `User.Roles` or migrating existing user documents — ops + concern; this PR adds the field and the reader, no migration. + +## Future optimizations + +- **Parallel auth + data fetch** via `errgroup.WithContext` — saves one + round-trip on the happy path; deferred for simplicity. +- **Response caching** (short TTL, per-site for tabs, per-room for + cmd-menus) — deferred until measured contention. + +## Risks + +- **`User.Roles` absence on legacy docs.** Decodes to nil → + `IsPlatformAdmin` returns false. Real admins lose access until ops + backfills via `updateMany`. +- **`Subscription.User.IsBot` absence on pre-#219 subs.** Bot subs + written before the field existed have `isBot=false` and miss the + `ListRoomBotApps` join. Same backfill class as `mute.toggle`. +- **`SITE_URL` misconfigured.** Caught at startup; service exits. +- **Response exceeds NATS `max_payload`.** Fails with + `errResponseTooLarge` (clean client error, not a silent drop). + Mitigation: tighten the offending menu, or follow up with a + per-assistant endpoint. +- **`bot_cmd_menu` schema drift.** No existing reader; the types here + are the canonical schema. diff --git a/pkg/model/app.go b/pkg/model/app.go index bd1327f7e..3fd6394f2 100644 --- a/pkg/model/app.go +++ b/pkg/model/app.go @@ -2,11 +2,13 @@ package model // App is a read-only view of the apps collection (provisioning is upstream). type App struct { - ID string `json:"id" bson:"_id"` - Name string `json:"name" bson:"name"` - Description string `json:"description,omitempty" bson:"description,omitempty"` - Assistant *AppAssistant `json:"assistant,omitempty" bson:"assistant,omitempty"` - Sponsors []AppSponsor `json:"sponsors,omitempty" bson:"sponsors,omitempty"` + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + AvatarURL string `json:"avatarUrl,omitempty" bson:"avatarUrl,omitempty"` + Assistant *AppAssistant `json:"assistant,omitempty" bson:"assistant,omitempty"` + ChannelTab *AppChannelTab `json:"channelTab,omitempty" bson:"channelTab,omitempty"` + Sponsors []AppSponsor `json:"sponsors,omitempty" bson:"sponsors,omitempty"` } // AppAssistant: Name is the bot user account (".bot" suffix); botDM requires Enabled==true. @@ -16,7 +18,53 @@ type AppAssistant struct { SettingsURL string `json:"settingsUrl,omitempty" bson:"settingsUrl,omitempty"` } +// AppChannelTab describes a tab that can be embedded into channel rooms. +// Default==true marks tabs that appear by default in every channel. +type AppChannelTab struct { + Enabled bool `json:"enabled" bson:"enabled"` + Default bool `json:"default" bson:"default"` + Name string `json:"name" bson:"name"` + URL AppChannelTabURL `json:"url" bson:"url"` +} + +// AppChannelTabURL holds the URL template. Default is the canonical form +// with literal ${roomId} / ${siteId} placeholders that room-service +// substitutes when building per-room tab URLs. +type AppChannelTabURL struct { + Default string `json:"default" bson:"default"` +} + type AppSponsor struct { Name string `json:"name" bson:"name"` Phone string `json:"phone" bson:"phone"` } + +// RoomApp is a single entry in GetRoomAppTabsResponse.Apps — derived +// from an apps document with the per-room tabUrl substituted in. +type RoomApp struct { + ID string `json:"id" bson:"-"` + Name string `json:"name" bson:"-"` // = apps.channelTab.name + TabURL string `json:"tabUrl" bson:"-"` // computed (scheme+host+path-prefix from SITE_URL, ${roomId}/${siteId} substituted) + Assistant *AppAssistant `json:"assistant,omitempty" bson:"-"` + AvatarURL string `json:"avatarUrl,omitempty" bson:"-"` +} + +// GetRoomAppTabsResponse is the response body for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.tabs RPC. +type GetRoomAppTabsResponse struct { + Apps []RoomApp `json:"apps" bson:"-"` +} + +// RoomAppAssistant is a single entry in +// GetRoomAppCommandMenuResponse.AppAssistants. +type RoomAppAssistant struct { + AppName string `json:"appName" bson:"-"` // = apps.name + Name string `json:"name" bson:"-"` // = apps.assistant.name (bot account) + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty" bson:"-"` +} + +// GetRoomAppCommandMenuResponse is the response body for the +// chat.user.{account}.request.room.{roomID}.{siteID}.app.cmd-menu RPC. +type GetRoomAppCommandMenuResponse struct { + AppAssistants []RoomAppAssistant `json:"appAssistants" bson:"-"` +} diff --git a/pkg/model/botcmdmenu.go b/pkg/model/botcmdmenu.go new file mode 100644 index 000000000..5c2bd13d0 --- /dev/null +++ b/pkg/model/botcmdmenu.go @@ -0,0 +1,34 @@ +package model + +// BotCmdMenu is a row in the bot_cmd_menu collection. Name matches an +// AppAssistant.Name (the bot account) and joins back to the owning App +// via that field. ActiveStatus gates whether the menu is currently +// exposed to clients. +type BotCmdMenu struct { + ID string `json:"id" bson:"_id"` + Name string `json:"name" bson:"name"` + ActiveStatus bool `json:"activeStatus" bson:"activeStatus"` + CmdBlocks []CmdBlock `json:"cmdBlocks,omitempty" bson:"cmdBlocks,omitempty"` +} + +// CmdBlock is the recursive building block of a bot command menu. A +// block either renders directly (Text+ActionType+Payload), opens a +// modal (Modal), or groups nested blocks (Blocks). Fields are +// optional so the schema can evolve without breaking the wire +// contract. +type CmdBlock struct { + Text string `json:"text,omitempty" bson:"text,omitempty"` + ActionType string `json:"actionType,omitempty" bson:"actionType,omitempty"` + Description string `json:"description,omitempty" bson:"description,omitempty"` + Payload string `json:"payload,omitempty" bson:"payload,omitempty"` + Modal *CmdModal `json:"modal,omitempty" bson:"modal,omitempty"` + Blocks []CmdBlock `json:"blocks,omitempty" bson:"blocks,omitempty"` +} + +// CmdModal carries the slash-style command + param a modal-triggering +// block invokes. CmdModal does NOT nest its own blocks — recursive +// rendering happens via CmdBlock.Blocks on the enclosing CmdBlock. +type CmdModal struct { + Command string `json:"command,omitempty" bson:"command,omitempty"` + Param string `json:"param,omitempty" bson:"param,omitempty"` +} diff --git a/pkg/model/member.go b/pkg/model/member.go index d8e788d67..d5d455b7b 100644 --- a/pkg/model/member.go +++ b/pkg/model/member.go @@ -55,8 +55,12 @@ type RoomMemberEntry struct { // ListRoomMembers is called with enrich=true. Elided from JSON when zero. EngName string `json:"engName,omitempty" bson:"-"` ChineseName string `json:"chineseName,omitempty" bson:"-"` + // Name is the app's display name for bot members (account matching + // the ".bot" suffix). For humans, EngName/ChineseName are populated + // and Name stays empty. Caller chooses display: name ?? engName ?? account. + Name string `json:"name,omitempty" bson:"-"` IsOwner bool `json:"isOwner,omitempty" bson:"-"` - SectName string `json:"sectName,omitempty" bson:"-"` + OrgName string `json:"orgName,omitempty" bson:"-"` MemberCount int `json:"memberCount,omitempty" bson:"-"` } diff --git a/pkg/model/model_test.go b/pkg/model/model_test.go index 2c42800ed..230416ac7 100644 --- a/pkg/model/model_test.go +++ b/pkg/model/model_test.go @@ -39,6 +39,37 @@ func TestUserJSON_WithSectAndDept(t *testing.T) { roundTrip(t, &u, &model.User{}) } +func TestUserJSON_RolesOmittedWhenEmpty(t *testing.T) { + u := model.User{ID: "u1", Account: "alice", SiteID: "site-a"} + data, err := json.Marshal(&u) + require.NoError(t, err) + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) + _, has := raw["roles"] + assert.False(t, has, "nil Roles must be omitted from JSON") +} + +func TestIsPlatformAdmin(t *testing.T) { + tests := []struct { + name string + u *model.User + want bool + }{ + {"nil receiver", nil, false}, + {"nil roles", &model.User{Account: "alice"}, false}, + {"empty roles", &model.User{Account: "alice", Roles: []model.UserRole{}}, false}, + {"user role only", &model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleUser}}, false}, + {"admin role present", &model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, true}, + {"admin among many", &model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleUser, model.UserRoleAdmin, "auditor"}}, true}, + {"case-sensitive (Admin not admin)", &model.User{Account: "alice", Roles: []model.UserRole{"Admin"}}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, model.IsPlatformAdmin(tt.u)) + }) + } +} + func TestRoomJSON(t *testing.T) { lastMsg := time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC) lastMention := time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC) @@ -506,6 +537,12 @@ func TestSubscriptionJSON(t *testing.T) { }) } +func TestIsRoomMember(t *testing.T) { + assert.False(t, model.IsRoomMember(nil), "nil sub is not a member") + assert.True(t, model.IsRoomMember(&model.Subscription{}), "zero-value sub is a member (no role gating)") + assert.True(t, model.IsRoomMember(&model.Subscription{RoomID: "r1"}), "populated sub is a member") +} + func TestSubscriptionJSON_ThreadUnreadOmittedAlertAlwaysPresent(t *testing.T) { s := model.Subscription{ ID: "s1", @@ -1498,7 +1535,7 @@ func TestRoomMemberEntry_DisplayFields_OmittedWhenZero(t *testing.T) { require.NoError(t, err) var got map[string]any require.NoError(t, json.Unmarshal(data, &got)) - for _, k := range []string{"engName", "chineseName", "isOwner", "sectName", "memberCount"} { + for _, k := range []string{"engName", "chineseName", "name", "isOwner", "orgName", "memberCount"} { _, present := got[k] assert.False(t, present, "display field %q should be omitted when zero", k) } @@ -1507,7 +1544,7 @@ func TestRoomMemberEntry_DisplayFields_OmittedWhenZero(t *testing.T) { func TestRoomMemberEntry_DisplayFields_NotPersistedToBSON(t *testing.T) { entry := model.RoomMemberEntry{ ID: "org-1", Type: model.RoomMemberOrg, - SectName: "Engineering", MemberCount: 42, + OrgName: "Engineering", MemberCount: 42, } data, err := bson.Marshal(&entry) require.NoError(t, err) @@ -1516,12 +1553,72 @@ func TestRoomMemberEntry_DisplayFields_NotPersistedToBSON(t *testing.T) { require.NoError(t, bson.Unmarshal(data, &got)) assert.Equal(t, "org-1", got["id"]) assert.Equal(t, "org", got["type"]) - for _, k := range []string{"engName", "chineseName", "isOwner", "sectName", "memberCount"} { + for _, k := range []string{"engName", "chineseName", "name", "isOwner", "orgName", "memberCount"} { _, present := got[k] assert.False(t, present, "display field %q must not be persisted to BSON", k) } } +func TestRoomMemberEntry_BotName_RoundTrip(t *testing.T) { + t.Run("bot member name round-trips via JSON", func(t *testing.T) { + entry := model.RoomMemberEntry{ + ID: "u-bot", + Type: model.RoomMemberIndividual, + Account: "weather.bot", + Name: "Weather App", + } + data, err := json.Marshal(&entry) + require.NoError(t, err) + + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) + assert.Equal(t, "Weather App", raw["name"]) + // Human display fields must be absent for a bot entry. + _, hasEngName := raw["engName"] + assert.False(t, hasEngName, "engName must be absent for bot entry") + _, hasChineseName := raw["chineseName"] + assert.False(t, hasChineseName, "chineseName must be absent for bot entry") + + var dst model.RoomMemberEntry + require.NoError(t, json.Unmarshal(data, &dst)) + assert.Equal(t, entry, dst) + }) + + t.Run("name not persisted to BSON", func(t *testing.T) { + entry := model.RoomMemberEntry{ + ID: "u-bot", + Type: model.RoomMemberIndividual, + Account: "weather.bot", + Name: "Weather App", + } + data, err := bson.Marshal(&entry) + require.NoError(t, err) + var got bson.M + require.NoError(t, bson.Unmarshal(data, &got)) + _, hasName := got["name"] + assert.False(t, hasName, "name must not be persisted to BSON") + }) + + t.Run("orgName round-trips via JSON", func(t *testing.T) { + entry := model.RoomMemberEntry{ + ID: "sect-eng", + Type: model.RoomMemberOrg, + OrgName: "Engineering", + } + data, err := json.Marshal(&entry) + require.NoError(t, err) + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) + assert.Equal(t, "Engineering", raw["orgName"]) + _, hasSectName := raw["sectName"] + assert.False(t, hasSectName, "sectName key must not appear (renamed to orgName)") + + var dst model.RoomMemberEntry + require.NoError(t, json.Unmarshal(data, &dst)) + assert.Equal(t, entry, dst) + }) +} + func TestOrgMemberJSON(t *testing.T) { m := model.OrgMember{ ID: "u-alice", @@ -2214,6 +2311,150 @@ func TestAppAssistantDisabledRoundtrip(t *testing.T) { assert.False(t, dst.Assistant.Enabled) } +func TestAppRoundtrip_WithChannelTabAndAvatar(t *testing.T) { + a := model.App{ + ID: "app1", + Name: "Calendar", + AvatarURL: "https://cdn.example.com/avatars/calendar.png", + Assistant: &model.AppAssistant{Enabled: true, Name: "calendar.bot"}, + ChannelTab: &model.AppChannelTab{ + Enabled: true, + Default: true, + Name: "Calendar", + URL: model.AppChannelTabURL{ + Default: "https://upstream.example.com/calendar/${roomId}/${siteId}/index", + }, + }, + } + var dst model.App + roundTrip(t, &a, &dst) + require.NotNil(t, dst.ChannelTab) + assert.True(t, dst.ChannelTab.Enabled) + assert.True(t, dst.ChannelTab.Default) + assert.Equal(t, "Calendar", dst.ChannelTab.Name) + assert.Equal(t, "https://upstream.example.com/calendar/${roomId}/${siteId}/index", + dst.ChannelTab.URL.Default) + assert.Equal(t, "https://cdn.example.com/avatars/calendar.png", dst.AvatarURL) +} + +func TestAppChannelTabRoundtrip(t *testing.T) { + tab := model.AppChannelTab{ + Enabled: true, + Default: false, + Name: "Notes", + URL: model.AppChannelTabURL{Default: "https://upstream/notes"}, + } + var dst model.AppChannelTab + roundTrip(t, &tab, &dst) +} + +func TestBotCmdMenuRoundtrip(t *testing.T) { + m := model.BotCmdMenu{ + ID: "bcm1", + Name: "weather.bot", + ActiveStatus: true, + CmdBlocks: []model.CmdBlock{ + {Text: "/weather", ActionType: "command", Payload: "weather"}, + }, + } + var dst model.BotCmdMenu + roundTrip(t, &m, &dst) + require.Len(t, dst.CmdBlocks, 1) + assert.Equal(t, "/weather", dst.CmdBlocks[0].Text) +} + +func TestBotCmdMenuRoundtrip_Inactive(t *testing.T) { + m := model.BotCmdMenu{ + ID: "bcm2", + Name: "weather.bot", + ActiveStatus: false, + } + var dst model.BotCmdMenu + roundTrip(t, &m, &dst) + assert.False(t, dst.ActiveStatus) + assert.Nil(t, dst.CmdBlocks) +} + +func TestCmdBlockRoundtrip_Recursive(t *testing.T) { + block := model.CmdBlock{ + Text: "menu", + ActionType: "open", + Description: "open the menu", + Modal: &model.CmdModal{ + Command: "menu.open", + Param: "weather", + }, + Blocks: []model.CmdBlock{ + {Text: "today", Payload: "today"}, + {Text: "tomorrow", Payload: "tomorrow", Blocks: []model.CmdBlock{ + {Text: "morning", Payload: "tomorrow.am"}, + }}, + }, + } + var dst model.CmdBlock + roundTrip(t, &block, &dst) + require.NotNil(t, dst.Modal) + assert.Equal(t, "menu.open", dst.Modal.Command) + assert.Equal(t, "weather", dst.Modal.Param) + require.Len(t, dst.Blocks, 2) + require.Len(t, dst.Blocks[1].Blocks, 1) + assert.Equal(t, "morning", dst.Blocks[1].Blocks[0].Text) +} + +func TestGetRoomAppTabsResponseRoundtrip(t *testing.T) { + src := model.GetRoomAppTabsResponse{ + Apps: []model.RoomApp{ + { + ID: "app1", + Name: "Calendar", + TabURL: "https://chat.example.com/calendar/r1/site-a/index", + Assistant: &model.AppAssistant{Enabled: true, Name: "cal.bot"}, + AvatarURL: "https://cdn/cal.png", + }, + }, + } + var dst model.GetRoomAppTabsResponse + roundTrip(t, &src, &dst) + require.Len(t, dst.Apps, 1) + assert.Equal(t, "Calendar", dst.Apps[0].Name) +} + +func TestGetRoomAppTabsResponse_EmptyIsArrayNotNull(t *testing.T) { + src := model.GetRoomAppTabsResponse{Apps: []model.RoomApp{}} + data, err := json.Marshal(&src) + require.NoError(t, err) + assert.Contains(t, string(data), `"apps":[]`) + assert.NotContains(t, string(data), `"apps":null`) +} + +func TestGetRoomAppCommandMenuResponseRoundtrip(t *testing.T) { + src := model.GetRoomAppCommandMenuResponse{ + AppAssistants: []model.RoomAppAssistant{ + { + AppName: "Weather Bot", + Name: "weather.bot", + CmdBlocks: []model.CmdBlock{ + {Text: "/forecast", Payload: "forecast"}, + }, + }, + }, + } + var dst model.GetRoomAppCommandMenuResponse + roundTrip(t, &src, &dst) + require.Len(t, dst.AppAssistants, 1) + assert.Equal(t, "weather.bot", dst.AppAssistants[0].Name) +} + +func TestGetRoomAppCommandMenuResponse_EmptyIsArrayNotNull(t *testing.T) { + src := model.GetRoomAppCommandMenuResponse{ + AppAssistants: []model.RoomAppAssistant{}, + } + data, err := json.Marshal(&src) + require.NoError(t, err) + assert.Contains(t, string(data), `"appAssistants":[]`) + assert.NotContains(t, string(data), `"appAssistants":null`) +} + func TestCreateRoomReplyRoundtrip(t *testing.T) { r := model.CreateRoomReply{ Status: model.CreateRoomReplyAccepted, diff --git a/pkg/model/subscription.go b/pkg/model/subscription.go index eea9205c5..d333c9ef3 100644 --- a/pkg/model/subscription.go +++ b/pkg/model/subscription.go @@ -74,6 +74,14 @@ type DMSubscription struct { HRInfo *SubscriptionHRInfo `json:"hrInfo,omitempty" bson:"hrInfo,omitempty"` } +// IsRoomMember reports whether sub represents an active membership. +// Returns false for nil so callers can pass the result of a store lookup +// that returned (nil, ErrSubscriptionNotFound) — the caller is expected +// to have already classified the error and set sub to nil on not-found. +func IsRoomMember(sub *Subscription) bool { + return sub != nil +} + // MessageThreadReadRequest is the body of the message.thread.read RPC. type MessageThreadReadRequest struct { ThreadID string `json:"threadId"` diff --git a/pkg/model/user.go b/pkg/model/user.go index 868f52bb3..0fee107be 100644 --- a/pkg/model/user.go +++ b/pkg/model/user.go @@ -24,3 +24,17 @@ type User struct { EmployeeID string `json:"employeeId" bson:"employeeId"` Roles []UserRole `json:"roles,omitempty" bson:"roles,omitempty"` } + +// IsPlatformAdmin reports whether u holds the platform admin role. +// Returns false for nil receivers and for users without the role. +func IsPlatformAdmin(u *User) bool { + if u == nil { + return false + } + for _, r := range u.Roles { + if r == UserRoleAdmin { + return true + } + } + return false +} diff --git a/pkg/subject/subject.go b/pkg/subject/subject.go index 01854344a..37d4d8012 100644 --- a/pkg/subject/subject.go +++ b/pkg/subject/subject.go @@ -14,10 +14,17 @@ func ParseUserRoomSubject(subj string) (account, roomID string, ok bool) { return "", "", false } account = parts[2] + if !isValidAccountToken(account) { + return "", "", false + } // Find "room" token after user position for i := 3; i < len(parts)-1; i++ { if parts[i] == "room" { - return account, parts[i+1], true + roomID = parts[i+1] + if !isValidAccountToken(roomID) { + return "", "", false + } + return account, roomID, true } } return "", "", false @@ -252,30 +259,42 @@ func MemberListWildcard(siteID string) string { return fmt.Sprintf("chat.user.*.request.room.*.%s.member.list", siteID) } -// OrgMembers builds the subject for listing members of an org. -func OrgMembers(account, orgID string) string { - return fmt.Sprintf("chat.user.%s.request.orgs.%s.members", account, orgID) +// OrgMembers builds the subject for listing members of an org. siteID +// selects which site's user directory to query — each site has its own +// users collection, so org membership is per-site. Token order matches +// the room-scoped builders ("identifier → site → action"). Panics on +// any token containing NATS wildcard characters. +func OrgMembers(account, orgID, siteID string) string { + if !isValidAccountToken(account) || !isValidAccountToken(orgID) || !isValidAccountToken(siteID) { + panic("invalid subject token: contains NATS wildcard characters") + } + return fmt.Sprintf("chat.user.%s.request.orgs.%s.%s.members", account, orgID, siteID) } -// OrgMembersWildcard is the subscription pattern for the list-org-members endpoint. -func OrgMembersWildcard() string { - return "chat.user.*.request.orgs.*.members" +// OrgMembersWildcard is the per-site subscription pattern for the +// list-org-members endpoint. +func OrgMembersWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.orgs.*.%s.members", siteID) } -// ParseOrgMembersSubject returns the orgID from a subject matching the -// pattern "chat.user.{account}.request.orgs.{orgId}.members". -// Tokens (by strings.Split on "."): [0]chat [1]user [2]{account} [3]request -// [4]orgs [5]{orgId} [6]members. orgID is at positional index 5. -func ParseOrgMembersSubject(subj string) (orgID string, ok bool) { +// ParseOrgMembersSubject returns (orgID, siteID) from a subject +// matching "chat.user.{account}.request.orgs.{orgId}.{siteId}.members". +// Tokens: [0]chat [1]user [2]{account} [3]request [4]orgs [5]{orgId} +// [6]{siteId} [7]members. Returns ok=false when any token contains +// NATS wildcard characters. +func ParseOrgMembersSubject(subj string) (orgID, siteID string, ok bool) { parts := strings.Split(subj, ".") - if len(parts) != 7 { - return "", false + if len(parts) != 8 { + return "", "", false } if parts[0] != "chat" || parts[1] != "user" || parts[3] != "request" || - parts[4] != "orgs" || parts[6] != "members" { - return "", false + parts[4] != "orgs" || parts[7] != "members" { + return "", "", false } - return parts[5], true + if !isValidAccountToken(parts[2]) || !isValidAccountToken(parts[5]) || !isValidAccountToken(parts[6]) { + return "", "", false + } + return parts[5], parts[6], true } func RoomCanonicalWildcard(siteID string) string { @@ -444,6 +463,36 @@ func FavoriteToggleWildcard(siteID string) string { return fmt.Sprintf("chat.user.*.request.room.*.%s.favorite.toggle", siteID) } +// RoomAppTabs returns the concrete subject for the GetRoomAppTabs RPC. +// Pair with RoomAppTabsWildcard for room-service's QueueSubscribe. +func RoomAppTabs(account, roomID, siteID string) string { + if !isValidAccountToken(account) { + panic("invalid account token: contains NATS wildcard characters") + } + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.tabs", account, roomID, siteID) +} + +// RoomAppTabsWildcard is the per-site subscription pattern for the +// GetRoomAppTabs RPC. +func RoomAppTabsWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.tabs", siteID) +} + +// RoomAppCmdMenu returns the concrete subject for the +// GetRoomAppCommandMenu RPC. +func RoomAppCmdMenu(account, roomID, siteID string) string { + if !isValidAccountToken(account) { + panic("invalid account token: contains NATS wildcard characters") + } + return fmt.Sprintf("chat.user.%s.request.room.%s.%s.app.cmd-menu", account, roomID, siteID) +} + +// RoomAppCmdMenuWildcard is the per-site subscription pattern for the +// GetRoomAppCommandMenu RPC. +func RoomAppCmdMenuWildcard(siteID string) string { + return fmt.Sprintf("chat.user.*.request.room.*.%s.app.cmd-menu", siteID) +} + // RoomCreate: client→room-service create subject; siteID is the requester's site. func RoomCreate(account, siteID string) string { return fmt.Sprintf("chat.user.%s.request.room.%s.create", account, siteID) diff --git a/pkg/subject/subject_test.go b/pkg/subject/subject_test.go index 0e4a6401e..fbfc80b52 100644 --- a/pkg/subject/subject_test.go +++ b/pkg/subject/subject_test.go @@ -70,10 +70,10 @@ func TestSubjectBuilders(t *testing.T) { "chat.user.alice.request.room.r1.site-a.member.list"}, {"MemberListWildcard", subject.MemberListWildcard("site-a"), "chat.user.*.request.room.*.site-a.member.list"}, - {"OrgMembers", subject.OrgMembers("alice", "sect-eng"), - "chat.user.alice.request.orgs.sect-eng.members"}, - {"OrgMembersWildcard", subject.OrgMembersWildcard(), - "chat.user.*.request.orgs.*.members"}, + {"OrgMembers", subject.OrgMembers("alice", "sect-eng", "site-a"), + "chat.user.alice.request.orgs.sect-eng.site-a.members"}, + {"OrgMembersWildcard", subject.OrgMembersWildcard("site-a"), + "chat.user.*.request.orgs.*.site-a.members"}, {"MsgThreadPattern", subject.MsgThreadPattern("site-a"), "chat.user.{account}.request.room.{roomID}.site-a.msg.thread"}, {"MsgThreadParentPattern", subject.MsgThreadParentPattern("site-a"), @@ -122,6 +122,14 @@ func TestSubjectBuilders(t *testing.T) { "chat.user.*.request.room.*.site-a.room.rename"}, {"RoomRestricted", subject.RoomRestricted("site-a"), "chat.server.request.room.site-a.restricted"}, + {"RoomAppTabs", subject.RoomAppTabs("alice", "r1", "site-a"), + "chat.user.alice.request.room.r1.site-a.app.tabs"}, + {"RoomAppTabsWildcard", subject.RoomAppTabsWildcard("site-a"), + "chat.user.*.request.room.*.site-a.app.tabs"}, + {"RoomAppCmdMenu", subject.RoomAppCmdMenu("alice", "r1", "site-a"), + "chat.user.alice.request.room.r1.site-a.app.cmd-menu"}, + {"RoomAppCmdMenuWildcard", subject.RoomAppCmdMenuWildcard("site-a"), + "chat.user.*.request.room.*.site-a.app.cmd-menu"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -181,6 +189,10 @@ func TestParseUserRoomSubject(t *testing.T) { {"too_short", "chat.user.alice", "", "", false}, {"no_room", "chat.user.alice.request.foo.bar", "", "", false}, {"bad_prefix", "foo.user.alice.room.r1", "", "", false}, + {"wildcard_account", "chat.user.*.room.r1.site-a.msg.send", "", "", false}, + {"wildcard_roomid", "chat.user.alice.room.*.site-a.msg.send", "", "", false}, + {"gt_wildcard_account", "chat.user.>.room.r1.site-a.msg.send", "", "", false}, + {"empty_account", "chat.user..room.r1.site-a.msg.send", "", "", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -261,26 +273,35 @@ func TestWildcardPatterns(t *testing.T) { func TestParseOrgMembersSubject(t *testing.T) { tests := []struct { - name string - subj string - wantOrg string - wantOK bool + name string + subj string + wantOrg string + wantSite string + wantOK bool }{ - {"valid", "chat.user.alice.request.orgs.sect-eng.members", "sect-eng", true}, - {"wrong prefix", "chat.user.alice.request.rooms.get.r1", "", false}, - {"wrong suffix", "chat.user.alice.request.orgs.sect-eng.other", "", false}, - {"too short", "chat.user.alice.request.orgs", "", false}, - {"too long", "chat.user.alice.request.orgs.sect-eng.members.x", "", false}, - {"empty", "", "", false}, + {"valid", "chat.user.alice.request.orgs.sect-eng.site-a.members", "sect-eng", "site-a", true}, + {"different site", "chat.user.bob.request.orgs.dept-hr.site-b.members", "dept-hr", "site-b", true}, + {"wrong prefix", "chat.user.alice.request.rooms.get.r1", "", "", false}, + {"wrong suffix", "chat.user.alice.request.orgs.sect-eng.site-a.other", "", "", false}, + {"too short (7-token old form)", "chat.user.alice.request.orgs.sect-eng.members", "", "", false}, + {"too long", "chat.user.alice.request.orgs.sect-eng.site-a.members.x", "", "", false}, + {"empty", "", "", "", false}, + {"wildcard account", "chat.user.*.request.orgs.sect-eng.site-a.members", "", "", false}, + {"wildcard orgID", "chat.user.alice.request.orgs.*.site-a.members", "", "", false}, + {"wildcard siteID", "chat.user.alice.request.orgs.sect-eng.*.members", "", "", false}, + {"multi-token wildcard account", "chat.user.>.request.orgs.sect-eng.site-a.members", "", "", false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, ok := subject.ParseOrgMembersSubject(tt.subj) + gotOrg, gotSite, ok := subject.ParseOrgMembersSubject(tt.subj) if ok != tt.wantOK { t.Fatalf("ok = %v, want %v", ok, tt.wantOK) } - if got != tt.wantOrg { - t.Errorf("orgID = %q, want %q", got, tt.wantOrg) + if gotOrg != tt.wantOrg { + t.Errorf("orgID = %q, want %q", gotOrg, tt.wantOrg) + } + if gotSite != tt.wantSite { + t.Errorf("siteID = %q, want %q", gotSite, tt.wantSite) } }) } @@ -648,6 +669,26 @@ func TestUserServiceBuildersRejectWildcardAccounts(t *testing.T) { } } +func TestRoomAppAndOrgMembersBuildersRejectWildcards(t *testing.T) { + builders := []struct { + name string + fn func() + }{ + {"RoomAppTabs star account", func() { subject.RoomAppTabs("*", "r1", "s1") }}, + {"RoomAppTabs gt account", func() { subject.RoomAppTabs(">", "r1", "s1") }}, + {"RoomAppCmdMenu star account", func() { subject.RoomAppCmdMenu("*", "r1", "s1") }}, + {"RoomAppCmdMenu gt account", func() { subject.RoomAppCmdMenu(">", "r1", "s1") }}, + {"OrgMembers star account", func() { subject.OrgMembers("*", "o1", "s1") }}, + {"OrgMembers star orgID", func() { subject.OrgMembers("a1", "*", "s1") }}, + {"OrgMembers gt siteID", func() { subject.OrgMembers("a1", "o1", ">") }}, + } + for _, b := range builders { + t.Run(b.name, func(t *testing.T) { + assert.Panics(t, b.fn) + }) + } +} + func TestParseUserSubject_RejectsWildcardAccount(t *testing.T) { bad := []string{ "chat.user.*.request.user.s1.status.getByName", @@ -672,6 +713,22 @@ func TestParseRoomSubject_RejectsWildcardAccount(t *testing.T) { } } +func TestRoomAppTabs_ParseUserRoomSubject(t *testing.T) { + subj := subject.RoomAppTabs("alice", "r1", "site-a") + account, roomID, ok := subject.ParseUserRoomSubject(subj) + assert.True(t, ok) + assert.Equal(t, "alice", account) + assert.Equal(t, "r1", roomID) +} + +func TestRoomAppCmdMenu_ParseUserRoomSubject(t *testing.T) { + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + account, roomID, ok := subject.ParseUserRoomSubject(subj) + assert.True(t, ok) + assert.Equal(t, "alice", account) + assert.Equal(t, "r1", roomID) +} + func TestUserServicePatternBuilders(t *testing.T) { tests := []struct { name string diff --git a/room-service/deploy/docker-compose.yml b/room-service/deploy/docker-compose.yml index 38bfe63a6..2d09d724f 100644 --- a/room-service/deploy/docker-compose.yml +++ b/room-service/deploy/docker-compose.yml @@ -46,6 +46,7 @@ services: - NATS_URL=nats://nats:4222 - NATS_CREDS_FILE=/etc/nats/backend.creds - SITE_ID=site-local + - SITE_URL=http://localhost:3000 - MONGO_URI=mongodb://mongodb:27017 - MONGO_DB=chat - MAX_ROOM_SIZE=1000 diff --git a/room-service/handler.go b/room-service/handler.go index 0134bba34..e03826803 100644 --- a/room-service/handler.go +++ b/room-service/handler.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "net/url" "slices" "strconv" "strings" @@ -49,9 +50,11 @@ type Handler struct { publishToStream func(ctx context.Context, subj string, data []byte, msgID string) error publishCore func(ctx context.Context, subj string, data []byte) error restrictedRoomMinMembers int + siteURL *url.URL + maxResponseBytes int64 } -func NewHandler(store RoomStore, keyStore RoomKeyStore, memberListClient MemberListClient, msgReader MessageReader, siteID string, maxRoomSize, maxBatchSize int, memberListTimeout time.Duration, restrictedRoomMinMembers int, publishToStream func(context.Context, string, []byte, string) error, publishCore func(context.Context, string, []byte) error) *Handler { +func NewHandler(store RoomStore, keyStore RoomKeyStore, memberListClient MemberListClient, msgReader MessageReader, siteID string, maxRoomSize, maxBatchSize int, memberListTimeout time.Duration, restrictedRoomMinMembers int, publishToStream func(context.Context, string, []byte, string) error, publishCore func(context.Context, string, []byte) error, siteURL *url.URL, maxResponseBytes int64) *Handler { return &Handler{ store: store, keyStore: keyStore, @@ -64,6 +67,8 @@ func NewHandler(store RoomStore, keyStore RoomKeyStore, memberListClient MemberL restrictedRoomMinMembers: restrictedRoomMinMembers, publishToStream: publishToStream, publishCore: publishCore, + siteURL: siteURL, + maxResponseBytes: maxResponseBytes, } } @@ -116,7 +121,7 @@ func (h *Handler) RegisterCRUD(nc *otelnats.Conn) error { if _, err := nc.QueueSubscribe(subject.MemberListWildcard(h.siteID), queue, h.natsListMembers); err != nil { return fmt.Errorf("subscribe member list: %w", err) } - if _, err := nc.QueueSubscribe(subject.OrgMembersWildcard(), queue, h.natsListOrgMembers); err != nil { + if _, err := nc.QueueSubscribe(subject.OrgMembersWildcard(h.siteID), queue, h.natsListOrgMembers); err != nil { return fmt.Errorf("subscribe org members: %w", err) } if _, err := nc.QueueSubscribe(subject.RoomKeyEnsure(h.siteID), queue, h.NatsHandleEnsureRoomKey); err != nil { @@ -137,6 +142,12 @@ func (h *Handler) RegisterCRUD(nc *otelnats.Conn) error { if _, err := nc.QueueSubscribe(subject.RoomRestricted(h.siteID), queue, h.natsRoomRestricted); err != nil { return fmt.Errorf("subscribe room restricted: %w", err) } + if _, err := nc.QueueSubscribe(subject.RoomAppTabsWildcard(h.siteID), queue, h.natsGetRoomAppTabs); err != nil { + return fmt.Errorf("subscribe app tabs: %w", err) + } + if _, err := nc.QueueSubscribe(subject.RoomAppCmdMenuWildcard(h.siteID), queue, h.natsGetRoomAppCommandMenu); err != nil { + return fmt.Errorf("subscribe app cmd-menu: %w", err) + } return nil } @@ -441,7 +452,7 @@ func (h *Handler) natsListOrgMembers(m otelnats.Msg) { } func (h *Handler) handleListOrgMembers(ctx context.Context, subj string) (model.ListOrgMembersResponse, error) { - orgID, ok := subject.ParseOrgMembersSubject(subj) + orgID, _, ok := subject.ParseOrgMembersSubject(subj) if !ok { return model.ListOrgMembersResponse{}, fmt.Errorf("invalid org-members subject") } @@ -1995,3 +2006,209 @@ func (h *Handler) handleFavoriteToggle(ctx context.Context, subj string, _ []byt return json.Marshal(model.FavoriteToggleResponse{Status: "ok", Favorite: sub.Favorite}) } + +// authorizeRoomAppRead allows the request iff the caller has a +// subscription in roomID OR is a platform admin in the local users +// collection AND the room actually exists. The room-existence check +// gates only the admin bypass — without it, an admin could query app +// metadata for a fabricated room ID and receive a plausible-looking +// response (e.g. a non-empty default-tabs list, or an empty cmd-menu +// list that looks like success). Cross-site admin authority is out of +// scope: an admin whose users document lives on a different site is +// denied. +func (h *Handler) authorizeRoomAppRead(ctx context.Context, account, roomID string) error { + sub, err := h.store.GetSubscription(ctx, account, roomID) + if err != nil && !errors.Is(err, model.ErrSubscriptionNotFound) { + return fmt.Errorf("check room membership: %w", err) + } + if model.IsRoomMember(sub) { + return nil + } + user, err := h.store.GetUser(ctx, account) + if err != nil && !errors.Is(err, ErrUserNotFound) { + return fmt.Errorf("check platform admin: %w", err) + } + if !model.IsPlatformAdmin(user) { + return errAppAccessDenied + } + // Admin bypass: verify the room exists before allowing the read. + // Without this, admins could query app metadata for fabricated room + // IDs and get plausible-looking responses. + if _, err := h.store.GetRoom(ctx, roomID); err != nil { + if errors.Is(err, mongo.ErrNoDocuments) { + return errAppAccessDenied + } + return fmt.Errorf("check room existence: %w", err) + } + return nil +} + +// buildTabURL applies the SITE_URL-based scheme/host/path-prefix +// rewrite and the ${roomId}/${siteId} substitution to a channelTab URL +// template. Returns (url, true) on success; (_, false) when the +// template is empty, unparseable, or when siteURL is nil or the IDs +// fail the URL-safety check. +func (h *Handler) buildTabURL(tmpl, roomID string) (string, bool) { + if tmpl == "" { + return "", false + } + if h.siteURL == nil { + return "", false + } + if !isURLSafeIDToken(roomID) || !isURLSafeIDToken(h.siteID) { + return "", false + } + // Substitute BEFORE parsing so url.URL.String() doesn't percent-encode + // the substituted values (roomID/siteID are URL-safe by construction). + tmpl = strings.ReplaceAll(tmpl, "${roomId}", roomID) + tmpl = strings.ReplaceAll(tmpl, "${siteId}", h.siteID) + u, err := url.Parse(tmpl) + if err != nil { + return "", false + } + joined := h.siteURL.JoinPath(u.Path) + joined.User = nil + joined.RawQuery = u.RawQuery + joined.Fragment = u.Fragment + return joined.String(), true +} + +func (h *Handler) handleGetRoomAppTabs(ctx context.Context, subj string, _ []byte) (model.GetRoomAppTabsResponse, error) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + account, roomID, ok := subject.ParseUserRoomSubject(subj) + if !ok { + return model.GetRoomAppTabsResponse{}, errcode.BadRequest("invalid request") + } + + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes( + attribute.String("room.id", roomID), + attribute.String("site.id", h.siteID), + attribute.String("account", account), + ) + } + + if err := h.authorizeRoomAppRead(ctx, account, roomID); err != nil { + return model.GetRoomAppTabsResponse{}, err + } + + apps, err := h.store.ListDefaultChannelTabApps(ctx) + if err != nil { + return model.GetRoomAppTabsResponse{}, fmt.Errorf("list default channel-tab apps: %w", err) + } + + out := make([]model.RoomApp, 0, len(apps)) + for i := range apps { + app := &apps[i] + if app.ChannelTab == nil { + slog.Warn("skipping app with nil ChannelTab", + "appId", app.ID, "roomId", roomID, + "requestId", natsutil.RequestIDFromContext(ctx)) + continue + } + tabURL, ok := h.buildTabURL(app.ChannelTab.URL.Default, roomID) + if !ok { + slog.Warn("skipping app with empty or unparseable channelTab url", + "appId", app.ID, "roomId", roomID, + "requestId", natsutil.RequestIDFromContext(ctx)) + continue + } + out = append(out, model.RoomApp{ + ID: app.ID, + Name: app.ChannelTab.Name, + TabURL: tabURL, + Assistant: app.Assistant, + AvatarURL: app.AvatarURL, + }) + } + return model.GetRoomAppTabsResponse{Apps: out}, nil +} + +func (h *Handler) handleGetRoomAppCommandMenu(ctx context.Context, subj string, _ []byte) (model.GetRoomAppCommandMenuResponse, error) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + account, roomID, ok := subject.ParseUserRoomSubject(subj) + if !ok { + return model.GetRoomAppCommandMenuResponse{}, errcode.BadRequest("invalid request") + } + + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes( + attribute.String("room.id", roomID), + attribute.String("site.id", h.siteID), + attribute.String("account", account), + ) + } + + if err := h.authorizeRoomAppRead(ctx, account, roomID); err != nil { + return model.GetRoomAppCommandMenuResponse{}, err + } + + bots, err := h.store.ListRoomBotApps(ctx, roomID) + if err != nil { + return model.GetRoomAppCommandMenuResponse{}, fmt.Errorf("list room bot apps: %w", err) + } + if span := trace.SpanFromContext(ctx); span.IsRecording() { + span.SetAttributes(attribute.Int("bot.count", len(bots))) + } + + if len(bots) == 0 { + return model.GetRoomAppCommandMenuResponse{ + AppAssistants: make([]model.RoomAppAssistant, 0), + }, nil + } + + names := make([]string, 0, len(bots)) + for _, b := range bots { + names = append(names, b.AssistantName) + } + menus, err := h.store.ListActiveCmdMenus(ctx, names) + if err != nil { + return model.GetRoomAppCommandMenuResponse{}, fmt.Errorf("list active cmd menus: %w", err) + } + byName := make(map[string][]model.CmdBlock, len(menus)) + for _, m := range menus { + byName[m.Name] = m.CmdBlocks + } + + out := make([]model.RoomAppAssistant, 0, len(bots)) + for _, b := range bots { + out = append(out, model.RoomAppAssistant{ + AppName: b.AppName, + Name: b.AssistantName, + CmdBlocks: byName[b.AssistantName], + }) + } + return model.GetRoomAppCommandMenuResponse{AppAssistants: out}, nil +} + +func (h *Handler) natsGetRoomAppCommandMenu(m otelnats.Msg) { + ctx, err := wrappedCtx(m) + if err != nil { + errnats.Reply(ctx, m.Msg, err) + return + } + resp, err := h.handleGetRoomAppCommandMenu(ctx, m.Msg.Subject, m.Msg.Data) + if err != nil { + errnats.Reply(ctx, m.Msg, err) + return + } + h.replyBoundedJSON(ctx, m.Msg, resp) +} + +func (h *Handler) natsGetRoomAppTabs(m otelnats.Msg) { + ctx, err := wrappedCtx(m) + if err != nil { + errnats.Reply(ctx, m.Msg, err) + return + } + resp, err := h.handleGetRoomAppTabs(ctx, m.Msg.Subject, m.Msg.Data) + if err != nil { + errnats.Reply(ctx, m.Msg, err) + return + } + h.replyBoundedJSON(ctx, m.Msg, resp) +} diff --git a/room-service/handler_test.go b/room-service/handler_test.go index cf5e847b9..9f8256ea8 100644 --- a/room-service/handler_test.go +++ b/room-service/handler_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "net/url" "strings" "testing" "time" @@ -470,7 +471,7 @@ func TestHandler_RemoveMember_SelfLeave_Success(t *testing.T) { publishedSubj = subj publishedData = data return nil - }, nil) + }, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) @@ -511,7 +512,7 @@ func TestHandler_RemoveMember_OrgOnly_Rejected(t *testing.T) { store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil) store.EXPECT().GetSubscriptionWithMembership(gomock.Any(), "r1", "alice"). Return(&SubscriptionWithMembership{Subscription: sub, HasOrgMembership: true}, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove(tc.requester, "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: tc.target}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -538,7 +539,7 @@ func TestHandler_RemoveMember_SelfLeave_NoOrgs_Allowed(t *testing.T) { handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, func(ctx context.Context, _ string, data []byte, _ string) error { publishedData = data return nil - }, nil) + }, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -574,7 +575,7 @@ func TestHandler_RemoveMember_LastOwner_Rejected(t *testing.T) { } store.EXPECT().CountMembersAndOwners(gomock.Any(), "r1"). Return(&RoomCounts{MemberCount: 3, OwnerCount: 1}, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove(tc.requester, "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -596,7 +597,7 @@ func TestHandler_RemoveMember_LastMember_Rejected(t *testing.T) { Return(&SubscriptionWithMembership{Subscription: sub, HasIndividualMembership: true}, nil) store.EXPECT().CountMembersAndOwners(gomock.Any(), "r1"). Return(&RoomCounts{MemberCount: 1, OwnerCount: 0}, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -625,7 +626,7 @@ func TestHandler_RemoveMember_OwnerRemovesOther_Success(t *testing.T) { handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, func(ctx context.Context, subj string, data []byte, _ string) error { publishedData = data return nil - }, nil) + }, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "bob"}) resp, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -649,7 +650,7 @@ func TestHandler_RemoveMember_NonOwnerRemovesOther_Rejected(t *testing.T) { store.EXPECT().GetSubscriptionWithMembership(gomock.Any(), "r1", "bob"). Return(&SubscriptionWithMembership{Subscription: targetSub, HasIndividualMembership: true}, nil) store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(requesterSub, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "bob"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -670,7 +671,7 @@ func TestHandler_RemoveMember_OwnerRemovesOrg_Success(t *testing.T) { handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, func(ctx context.Context, subj string, data []byte, _ string) error { publishedData = data return nil - }, nil) + }, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", OrgID: "eng-org"}) resp, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -685,7 +686,7 @@ func TestHandler_RemoveMember_BothAccountAndOrgID_Rejected(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "bob", OrgID: "eng-org"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -697,7 +698,7 @@ func TestHandler_RemoveMember_NeitherAccountNorOrgID_Rejected(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") reqBody, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, reqBody) @@ -708,7 +709,7 @@ func TestHandler_RemoveMember_NeitherAccountNorOrgID_Rejected(t *testing.T) { func TestHandler_RemoveMember_InvalidSubject(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) _, err := handler.handleRemoveMember(context.Background(), "bogus", []byte("{}")) require.Error(t, err) assert.Contains(t, err.Error(), "invalid remove-member subject") @@ -717,7 +718,7 @@ func TestHandler_RemoveMember_InvalidSubject(t *testing.T) { func TestHandler_RemoveMember_InvalidJSON(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") _, err := handler.handleRemoveMember(context.Background(), reqSubj, []byte("{not json")) require.Error(t, err) @@ -727,7 +728,7 @@ func TestHandler_RemoveMember_InvalidJSON(t *testing.T) { func TestHandler_RemoveMember_RoomIDMismatch(t *testing.T) { ctrl := gomock.NewController(t) store := NewMockRoomStore(ctrl) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r2", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -741,7 +742,7 @@ func TestHandler_RemoveMember_GetTargetError(t *testing.T) { store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil) store.EXPECT().GetSubscriptionWithMembership(gomock.Any(), "r1", "alice"). Return(nil, fmt.Errorf("db down")) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -760,7 +761,7 @@ func TestHandler_RemoveMember_OwnerRemoves_RequesterLookupError(t *testing.T) { Return(&SubscriptionWithMembership{Subscription: targetSub, HasIndividualMembership: true}, nil) store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). Return(nil, fmt.Errorf("db down")) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "bob"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -779,7 +780,7 @@ func TestHandler_RemoveMember_CountsError(t *testing.T) { Return(&SubscriptionWithMembership{Subscription: sub, HasIndividualMembership: true}, nil) store.EXPECT().CountMembersAndOwners(gomock.Any(), "r1"). Return(nil, fmt.Errorf("db down")) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -793,7 +794,7 @@ func TestHandler_RemoveMember_OrgPath_RequesterLookupError(t *testing.T) { store.EXPECT().GetRoom(gomock.Any(), "r1").Return(&model.Room{ID: "r1", Type: model.RoomTypeChannel}, nil) store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). Return(nil, fmt.Errorf("db down")) - handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil) + handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", OrgID: "eng-org"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -814,7 +815,7 @@ func TestHandler_RemoveMember_PublishError(t *testing.T) { Return(&RoomCounts{MemberCount: 3, OwnerCount: 2}, nil) handler := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, func(_ context.Context, _ string, _ []byte, _ string) error { return fmt.Errorf("nats down") - }, nil) + }, nil, nil, 0) reqSubj := subject.MemberRemove("alice", "r1", "site-a") body, _ := json.Marshal(model.RemoveMemberRequest{RoomID: "r1", Account: "alice"}) _, err := handler.handleRemoveMember(context.Background(), reqSubj, body) @@ -926,7 +927,7 @@ func TestHandler_AddMembers_RestrictedOwnerAllowed(t *testing.T) { store := NewMockRoomStore(ctrl) publish := func(_ context.Context, _ string, _ []byte, _ string) error { return nil } - h := NewHandler(store, nil, nil, nil, "site-a", 100, 500, 5*time.Second, 5, publish, nil) + h := NewHandler(store, nil, nil, nil, "site-a", 100, 500, 5*time.Second, 5, publish, nil, nil, 0) store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{ Roles: []model.Role{model.RoleOwner}, @@ -954,7 +955,7 @@ func TestHandler_AddMembers_EmptyAfterResolve(t *testing.T) { store := NewMockRoomStore(ctrl) publish := func(_ context.Context, _ string, _ []byte, _ string) error { return nil } - h := NewHandler(store, nil, nil, nil, "site-a", 100, 500, 5*time.Second, 5, publish, nil) + h := NewHandler(store, nil, nil, nil, "site-a", 100, 500, 5*time.Second, 5, publish, nil, nil, 0) store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1").Return(&model.Subscription{ Roles: []model.Role{model.RoleMember}, @@ -1790,7 +1791,7 @@ func TestHandler_ListMembers(t *testing.T) { func TestHandler_ListOrgMembers(t *testing.T) { const orgID = "sect-eng" - subj := subject.OrgMembers("alice", orgID) + subj := subject.OrgMembers("alice", orgID, "site-a") members := []model.OrgMember{ {ID: "u-a", Account: "a", EngName: "A", ChineseName: "AA", SiteID: "site-a"}, @@ -3302,7 +3303,7 @@ func TestHandler_handleMessageReadReceipt(t *testing.T) { tt.prep(setup{store: store, reader: reader}) } - h := NewHandler(store, nil, nil, reader, siteID, 1000, 1000, time.Second, 5, nil, nil) + h := NewHandler(store, nil, nil, reader, siteID, 1000, 1000, time.Second, 5, nil, nil, nil, 0) gotBytes, err := h.handleMessageReadReceipt(context.Background(), tt.subject, tt.body) if tt.wantErr != nil { @@ -4158,7 +4159,7 @@ func TestHandler_natsGetRoomKey(t *testing.T) { ks := NewMockRoomKeyStore(ctrl) tc.setup(t, store, ks) - h := NewHandler(store, ks, nil, nil, siteID, 1000, 500, 5*time.Second, 5, nil, nil) + h := NewHandler(store, ks, nil, nil, siteID, 1000, 500, 5*time.Second, 5, nil, nil, nil, 0) resp, err := h.handleGetRoomKey(t.Context(), subj, tc.body) if tc.want.errSubstr != "" { require.Error(t, err) @@ -4292,7 +4293,7 @@ func TestHandleRoomRename_Validation(t *testing.T) { tt.setupStore(store) } h := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, - func(_ context.Context, _ string, _ []byte, _ string) error { return nil }, nil) + func(_ context.Context, _ string, _ []byte, _ string) error { return nil }, nil, nil, 0) _, err := h.handleRoomRename(tt.ctx, tt.subj, tt.body) if tt.wantErr == nil { @@ -4455,7 +4456,7 @@ func TestHandleRoomRestricted_Validation(t *testing.T) { tt.setupStore(store) } h := NewHandler(store, nil, nil, nil, "site-a", 1000, 500, 5*time.Second, 5, - func(_ context.Context, _ string, _ []byte, _ string) error { return nil }, nil) + func(_ context.Context, _ string, _ []byte, _ string) error { return nil }, nil, nil, 0) _, err := h.handleRoomRestricted(tt.ctx, tt.body) if tt.wantErr == nil { @@ -4810,3 +4811,588 @@ func TestHandler_FavoriteToggle_CorePublishFailureIsNonFatal(t *testing.T) { assert.Equal(t, "ok", got.Status) assert.True(t, got.Favorite) } + +func TestHandler_marshalBounded(t *testing.T) { + type sample struct { + Hello string `json:"hello"` + } + big := sample{Hello: strings.Repeat("x", 200)} + small := sample{Hello: "hi"} + + tests := []struct { + name string + maxResponseBytes int64 + value any + wantBodyEmpty bool + wantErr error // errors.Is target; nil = expect no error + wantErrNonNil bool // expect a non-nil error with no specific sentinel + }{ + {"under cap", 1024, small, false, nil, false}, + {"over cap", 64, big, true, errResponseTooLarge, true}, + {"disabled zero", 0, big, false, nil, false}, + {"disabled negative", -1, big, false, nil, false}, + {"marshal failure", 1024, func() {}, true, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := &Handler{maxResponseBytes: tt.maxResponseBytes} + body, err := h.marshalBounded(tt.value) + if tt.wantErrNonNil { + require.Error(t, err) + } else { + require.NoError(t, err) + } + if tt.wantErr != nil { + assert.ErrorIs(t, err, tt.wantErr) + } + if tt.wantBodyEmpty { + assert.Nil(t, body) + } else { + assert.NotEmpty(t, body) + } + }) + } +} + +func TestHandler_authorizeRoomAppRead(t *testing.T) { + tests := []struct { + name string + setupMock func(*MockRoomStore) + wantErr error + wantErrContains string + }{ + { + name: "member allowed", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{ + User: model.SubscriptionUser{ID: "u1", Account: "alice"}, + RoomID: "r1", + }, nil) + }, + wantErr: nil, + }, + { + name: "admin allowed (no sub, admin role, room exists)", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) + s.EXPECT().GetRoom(gomock.Any(), "r1"). + Return(&model.Room{ID: "r1"}, nil) + }, + wantErr: nil, + }, + { + name: "denied: admin role but room does not exist", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) + s.EXPECT().GetRoom(gomock.Any(), "r1"). + Return(nil, mongo.ErrNoDocuments) + }, + wantErr: errAppAccessDenied, + }, + { + name: "denied: no sub, no admin role", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []model.UserRole{model.UserRoleUser}}, nil) + }, + wantErr: errAppAccessDenied, + }, + { + name: "denied: no sub, user not found (cross-site admin path)", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, ErrUserNotFound) + }, + wantErr: errAppAccessDenied, + }, + { + name: "transient sub-check error propagates", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, errors.New("mongo unavailable")) + }, + wantErrContains: "check room membership", + }, + { + name: "transient user-lookup error propagates", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, errors.New("mongo unavailable")) + }, + wantErrContains: "check platform admin", + }, + { + name: "transient room-existence error propagates (admin path)", + setupMock: func(s *MockRoomStore) { + s.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + s.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{ID: "u1", Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) + s.EXPECT().GetRoom(gomock.Any(), "r1"). + Return(nil, errors.New("mongo unavailable")) + }, + wantErrContains: "check room existence", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + tt.setupMock(store) + h := &Handler{store: store, siteID: "site-a"} + err := h.authorizeRoomAppRead(context.Background(), "alice", "r1") + switch { + case tt.wantErrContains != "": + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErrContains) + case tt.wantErr == nil: + assert.NoError(t, err) + default: + assert.ErrorIs(t, err, tt.wantErr) + } + }) + } +} + +func newTabsTestHandler(t *testing.T, siteURL string) (*Handler, *MockRoomStore, *gomock.Controller) { + t.Helper() + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + u, err := url.Parse(siteURL) + require.NoError(t, err) + return &Handler{store: store, siteID: "site-a", siteURL: u}, store, ctrl +} + +func mockTabApp(id, tabName, urlTemplate string) model.App { + return model.App{ + ID: id, + AvatarURL: "https://cdn/" + id + ".png", + Assistant: &model.AppAssistant{Enabled: true, Name: id + ".bot"}, + ChannelTab: &model.AppChannelTab{ + Enabled: true, Default: true, Name: tabName, + URL: model.AppChannelTabURL{Default: urlTemplate}, + }, + } +} + +func TestHandler_buildTabURL(t *testing.T) { + validSiteURL, err := url.Parse("https://chat.example.com") + require.NoError(t, err) + + tests := []struct { + name string + handler *Handler + tmpl string + roomID string + wantURL string + wantOK bool + }{ + { + name: "happy path", + handler: &Handler{siteID: "site-a", siteURL: validSiteURL}, + tmpl: "https://upstream/tab/${roomId}/${siteId}", + roomID: "r1", + wantURL: "https://chat.example.com/tab/r1/site-a", + wantOK: true, + }, + { + name: "empty template", + handler: &Handler{siteID: "site-a", siteURL: validSiteURL}, + tmpl: "", + roomID: "r1", + wantOK: false, + }, + { + name: "nil siteURL", + handler: &Handler{siteID: "site-a"}, + tmpl: "https://upstream/tab/${roomId}", + roomID: "r1", + wantOK: false, + }, + { + name: "non-URL-safe roomID", + handler: &Handler{siteID: "site-a", siteURL: validSiteURL}, + tmpl: "https://upstream/tab/${roomId}", + roomID: "r1/../../etc", + wantOK: false, + }, + { + name: "non-URL-safe siteID", + handler: &Handler{siteID: "site/../a", siteURL: validSiteURL}, + tmpl: "https://upstream/tab/${roomId}", + roomID: "r1", + wantOK: false, + }, + { + name: "malformed template", + handler: &Handler{siteID: "site-a", siteURL: validSiteURL}, + tmpl: "://malformed", + roomID: "r1", + wantOK: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := tt.handler.buildTabURL(tt.tmpl, tt.roomID) + assert.Equal(t, tt.wantOK, ok) + assert.Equal(t, tt.wantURL, got) + }) + } +} + +func TestHandler_handleGetRoomAppTabs_MemberAllowed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{ID: "u1", Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "Calendar", "https://upstream/cal/${roomId}/${siteId}/index"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.Apps, 1) + assert.Equal(t, "app1", resp.Apps[0].ID) + assert.Equal(t, "Calendar", resp.Apps[0].Name) + assert.Equal(t, "https://chat.example.com/cal/r1/site-a/index", resp.Apps[0].TabURL) + assert.Equal(t, "https://cdn/app1.png", resp.Apps[0].AvatarURL) + require.NotNil(t, resp.Apps[0].Assistant) + assert.Equal(t, "app1.bot", resp.Apps[0].Assistant.Name) +} + +func TestHandler_handleGetRoomAppTabs_AdminAllowed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) + store.EXPECT().GetRoom(gomock.Any(), "r1"). + Return(&model.Room{ID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{}, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Empty(t, resp.Apps) +} + +func TestHandler_handleGetRoomAppTabs_Denied(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleUser}}, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppTabs_DeniedNoUser(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(nil, ErrUserNotFound) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppTabs_EmptyResultIsEmptyArray(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return(nil, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotNil(t, resp.Apps, "must initialize empty slice, not nil, so JSON marshals to []") + assert.Len(t, resp.Apps, 0) + data, err := json.Marshal(resp) + require.NoError(t, err) + assert.Contains(t, string(data), `"apps":[]`) +} + +func TestHandler_handleGetRoomAppTabs_URLRewritePathPrefix(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com/chat") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "Calendar", "https://upstream/tab/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Equal(t, "https://chat.example.com/chat/tab/r1", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewriteStripsUserinfo(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "X", "https://user:pass@upstream/path/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotContains(t, resp.Apps[0].TabURL, "user") + assert.NotContains(t, resp.Apps[0].TabURL, "pass") + assert.Equal(t, "https://chat.example.com/path/r1", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewritePreservesQueryAndFragment(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("app1", "X", "https://upstream/path?room=${roomId}#tab=${siteId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + assert.Equal(t, "https://chat.example.com/path?room=r1#tab=site-a", resp.Apps[0].TabURL) +} + +func TestHandler_handleGetRoomAppTabs_URLRewriteSkipsEmptyAndMalformed(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + mockTabApp("ok1", "OK1", "https://upstream/ok1/${roomId}"), + mockTabApp("empty", "Empty", ""), + mockTabApp("bad", "Bad", "://malformed"), + mockTabApp("ok2", "OK2", "https://upstream/ok2/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.Apps, 2, "empty and malformed must be skipped") + assert.Equal(t, "ok1", resp.Apps[0].ID) + assert.Equal(t, "ok2", resp.Apps[1].ID) +} + +func TestHandler_handleGetRoomAppTabs_SkipsAppWithNilChannelTab(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + // One app has nil ChannelTab (invalid data), one is valid — only the valid one should appear. + appNoTab := model.App{ID: "notab", AvatarURL: "https://cdn/notab.png", ChannelTab: nil} + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()).Return([]model.App{ + appNoTab, + mockTabApp("ok1", "OK1", "https://upstream/ok1/${roomId}"), + }, nil) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.Apps, 1, "app with nil ChannelTab must be skipped") + assert.Equal(t, "ok1", resp.Apps[0].ID) +} + +func TestHandler_handleGetRoomAppTabs_InvalidSubject(t *testing.T) { + h, _, _ := newTabsTestHandler(t, "https://chat.example.com") + _, err := h.handleGetRoomAppTabs(context.Background(), "not.a.valid.subject", nil) + assert.Error(t, err) +} + +func TestHandler_handleGetRoomAppTabs_StoreListError(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListDefaultChannelTabApps(gomock.Any()). + Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppTabs_ContextTimeout(t *testing.T) { + h, store, _ := newTabsTestHandler(t, "https://chat.example.com") + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + DoAndReturn(func(ctx context.Context, _, _ string) (*model.Subscription, error) { + <-ctx.Done() + return nil, ctx.Err() + }) + + parent, cancel := context.WithCancel(context.Background()) + cancel() + subj := subject.RoomAppTabs("alice", "r1", "site-a") + _, err := h.handleGetRoomAppTabs(parent, subj, nil) + require.Error(t, err) + assert.True(t, + errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded), + "expected wrapped context error, got %v", err) +} + +func newCmdMenuTestHandler(t *testing.T) (*Handler, *MockRoomStore) { + t.Helper() + ctrl := gomock.NewController(t) + store := NewMockRoomStore(ctrl) + return &Handler{store: store, siteID: "site-a"}, store +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_NoBots(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{}, nil) + // ListActiveCmdMenus must NOT be called. + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + assert.NotNil(t, resp.AppAssistants) + assert.Len(t, resp.AppAssistants, 0) + data, err := json.Marshal(resp) + require.NoError(t, err) + assert.Contains(t, string(data), `"appAssistants":[]`) +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_WithMenus(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "stocks.bot", AppName: "Stocks"}, + {AssistantName: "weather.bot", AppName: "Weather"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"stocks.bot", "weather.bot"}). + Return([]model.BotCmdMenu{ + {Name: "stocks.bot", CmdBlocks: []model.CmdBlock{{Text: "/quote"}}}, + {Name: "weather.bot", CmdBlocks: []model.CmdBlock{{Text: "/forecast"}}}, + }, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.AppAssistants, 2) + assert.Equal(t, "Stocks", resp.AppAssistants[0].AppName) + assert.Equal(t, "stocks.bot", resp.AppAssistants[0].Name) + require.Len(t, resp.AppAssistants[0].CmdBlocks, 1) + assert.Equal(t, "/quote", resp.AppAssistants[0].CmdBlocks[0].Text) + assert.Equal(t, "Weather", resp.AppAssistants[1].AppName) + assert.Equal(t, "/forecast", resp.AppAssistants[1].CmdBlocks[0].Text) +} + +func TestHandler_handleGetRoomAppCommandMenu_MemberAllowed_BotWithoutMenu(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "silent.bot", AppName: "Silent"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"silent.bot"}). + Return([]model.BotCmdMenu{}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + require.Len(t, resp.AppAssistants, 1) + assert.Equal(t, "Silent", resp.AppAssistants[0].AppName) + assert.Equal(t, "silent.bot", resp.AppAssistants[0].Name) + assert.Nil(t, resp.AppAssistants[0].CmdBlocks) +} + +func TestHandler_handleGetRoomAppCommandMenu_AdminAllowed(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, nil) + store.EXPECT().GetRoom(gomock.Any(), "r1"). + Return(&model.Room{ID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + resp, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.NoError(t, err) + assert.Len(t, resp.AppAssistants, 0) +} + +func TestHandler_handleGetRoomAppCommandMenu_Denied(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(nil, model.ErrSubscriptionNotFound) + store.EXPECT().GetUser(gomock.Any(), "alice"). + Return(&model.User{Account: "alice"}, nil) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + assert.ErrorIs(t, err, errAppAccessDenied) +} + +func TestHandler_handleGetRoomAppCommandMenu_InvalidSubject(t *testing.T) { + h, _ := newCmdMenuTestHandler(t) + _, err := h.handleGetRoomAppCommandMenu(context.Background(), "not.a.valid.subject", nil) + assert.Error(t, err) +} + +func TestHandler_handleGetRoomAppCommandMenu_StoreListRoomBotAppsError(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppCommandMenu_StoreListActiveCmdMenusError(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + Return(&model.Subscription{User: model.SubscriptionUser{Account: "alice"}, RoomID: "r1"}, nil) + store.EXPECT().ListRoomBotApps(gomock.Any(), "r1").Return([]RoomBotAppEntry{ + {AssistantName: "weather.bot", AppName: "Weather"}, + }, nil) + store.EXPECT().ListActiveCmdMenus(gomock.Any(), []string{"weather.bot"}). + Return(nil, errors.New("mongo down")) + + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(context.Background(), subj, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "mongo down") +} + +func TestHandler_handleGetRoomAppCommandMenu_ContextTimeout(t *testing.T) { + h, store := newCmdMenuTestHandler(t) + store.EXPECT().GetSubscription(gomock.Any(), "alice", "r1"). + DoAndReturn(func(ctx context.Context, _, _ string) (*model.Subscription, error) { + <-ctx.Done() + return nil, ctx.Err() + }) + + parent, cancel := context.WithCancel(context.Background()) + cancel() + subj := subject.RoomAppCmdMenu("alice", "r1", "site-a") + _, err := h.handleGetRoomAppCommandMenu(parent, subj, nil) + require.Error(t, err) + assert.True(t, + errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded), + "expected wrapped context error, got %v", err) +} diff --git a/room-service/helper.go b/room-service/helper.go index 2bc9b1d45..6002882db 100644 --- a/room-service/helper.go +++ b/room-service/helper.go @@ -2,10 +2,16 @@ package main import ( "context" + "encoding/json" + "fmt" + "log/slog" "regexp" "time" + "github.com/nats-io/nats.go" + "github.com/hmchangw/chat/pkg/errcode" + "github.com/hmchangw/chat/pkg/errcode/errnats" "github.com/hmchangw/chat/pkg/model" ) @@ -84,6 +90,11 @@ var ( errRoomNotFound = errcode.NotFound("room not found") errInvalidRenameSubject = errcode.BadRequest("invalid rename subject") errInvalidRestrictedSubject = errcode.BadRequest("invalid restricted subject") + // App-read RPC sentinels (app.tabs / app.cmd-menu). Forbidden when the + // caller is neither a room member nor a platform admin; Internal when a + // reply would exceed the negotiated NATS max_payload. + errAppAccessDenied = errcode.Forbidden("not authorized to access this room's apps", errcode.WithReason(errcode.RoomNotMember)) + errResponseTooLarge = errcode.Internal("response payload exceeds maximum size") ) var botPattern = regexp.MustCompile(`\.bot$|^p_`) @@ -187,3 +198,54 @@ func stripAccount(slice []string, account string) []string { } return out } + +// marshalBounded marshals v and enforces h.maxResponseBytes (<= 0 disables +// the bound). Returns (body, nil) on success, or (nil, err) suitable for +// errnats.Reply: errResponseTooLarge on a size violation, a wrapped marshal +// error (classified to a generic internal error) on a marshal failure. +func (h *Handler) marshalBounded(v any) ([]byte, error) { + body, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("marshal bounded response: %w", err) + } + if h.maxResponseBytes > 0 && int64(len(body)) > h.maxResponseBytes { + return nil, errResponseTooLarge + } + return body, nil +} + +// replyBoundedJSON sends v on msg's reply subject, enforcing the negotiated +// NATS max_payload. nats* handlers use this in place of natsutil.ReplyJSON +// when a response payload could exceed max_payload; an oversize payload is +// surfaced to the caller via errnats.Reply rather than silently dropped. +func (h *Handler) replyBoundedJSON(ctx context.Context, msg *nats.Msg, v any) { + body, err := h.marshalBounded(v) + if err != nil { + errnats.Reply(ctx, msg, err) + return + } + if err := msg.Respond(body); err != nil { + slog.ErrorContext(ctx, "reply failed", "error", err) + } +} + +// isURLSafeIDToken reports whether s is safe to inline into a URL +// template path without risk of injection. Allows alphanumerics, +// hyphen, underscore, dot, and tilde (RFC3986 unreserved); rejects +// characters that could escape the path segment (?, #, /, etc.). +func isURLSafeIDToken(s string) bool { + if s == "" { + return false + } + for _, r := range s { + switch { + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + case r == '-' || r == '_' || r == '.' || r == '~': + default: + return false + } + } + return true +} diff --git a/room-service/helper_test.go b/room-service/helper_test.go index c2a3aad10..36fba4aaa 100644 --- a/room-service/helper_test.go +++ b/room-service/helper_test.go @@ -173,6 +173,37 @@ func TestIsPlatformAdmin(t *testing.T) { } } +func TestIsURLSafeIDToken(t *testing.T) { + tests := []struct { + name string + input string + want bool + }{ + {"empty string", "", false}, + {"simple alphanumeric", "site-a", true}, + {"lowercase", "roomabc123", true}, + {"uppercase", "SiteA", true}, + {"underscore", "room_id", true}, + {"dot", "v1.2.3", true}, + {"tilde", "abc~def", true}, + {"hyphen", "room-id-123", true}, + {"question mark", "room?id", false}, + {"hash", "room#id", false}, + {"slash", "room/id", false}, + {"asterisk", "room*", false}, + {"greater than", "room>id", false}, + {"less than", "room 0 { + u, err := s.findUsersForDisplay(ctx, humanAccounts) + if err != nil { + return fmt.Errorf("find users for room %q: %w", roomID, err) + } + userByAccount = u + } + if len(botAccounts) > 0 { + a, err := s.findAppsForDisplay(ctx, botAccounts) + if err != nil { + return fmt.Errorf("find apps for room %q: %w", roomID, err) + } + appByAssistant = a } + + for i := range members { + if members[i].Member.Type != model.RoomMemberIndividual { + continue + } + acct := members[i].Member.Account + if u, ok := userByAccount[acct]; ok { + members[i].Member.EngName = u.EngName + members[i].Member.ChineseName = u.ChineseName + continue + } + if name, ok := appByAssistant[acct]; ok { + members[i].Member.Name = name + } + } + return nil +} + +// findUsersForDisplay returns engName/chineseName indexed by account +// for every users document matching one of accounts. The existing +// users.account index covers the $in filter. +func (s *MongoStore) findUsersForDisplay(ctx context.Context, accounts []string) (map[string]*model.User, error) { cursor, err := s.users.Find(ctx, bson.M{"account": bson.M{"$in": accounts}}, options.Find().SetProjection(bson.M{"_id": 0, "account": 1, "engName": 1, "chineseName": 1}), ) if err != nil { - return fmt.Errorf("find users for %q: %w", roomID, err) + return nil, fmt.Errorf("find users for display: %w", err) } defer cursor.Close(ctx) var users []model.User if err := cursor.All(ctx, &users); err != nil { - return fmt.Errorf("decode users for %q: %w", roomID, err) + return nil, fmt.Errorf("decode users for display: %w", err) } - byAccount := make(map[string]*model.User, len(users)) + out := make(map[string]*model.User, len(users)) for i := range users { - byAccount[users[i].Account] = &users[i] + out[users[i].Account] = &users[i] } - for i := range members { - if u, ok := byAccount[members[i].Member.Account]; ok { - members[i].Member.EngName = u.EngName - members[i].Member.ChineseName = u.ChineseName - } + return out, nil +} + +// findAppsForDisplay returns app.name indexed by assistant.name for +// every apps document whose assistant.name matches one of botAccounts. +// The existing apps (assistant.name) index covers the $in filter. +func (s *MongoStore) findAppsForDisplay(ctx context.Context, botAccounts []string) (map[string]string, error) { + cursor, err := s.apps.Find(ctx, + bson.M{"assistant.name": bson.M{"$in": botAccounts}}, + options.Find().SetProjection(bson.M{"_id": 0, "name": 1, "assistant.name": 1}), + ) + if err != nil { + return nil, fmt.Errorf("find apps for display: %w", err) } - return nil + defer cursor.Close(ctx) + + type row struct { + Name string `bson:"name"` + Assistant struct { + Name string `bson:"name"` + } `bson:"assistant"` + } + var rows []row + if err := cursor.All(ctx, &rows); err != nil { + return nil, fmt.Errorf("decode apps for display: %w", err) + } + out := make(map[string]string, len(rows)) + for _, r := range rows { + out[r.Assistant.Name] = r.Name + } + return out, nil } func (s *MongoStore) GetUser(ctx context.Context, account string) (*model.User, error) { @@ -1027,6 +1122,101 @@ func (s *MongoStore) UpdateSubscriptionThreadRead(ctx context.Context, roomID, a return nil } +// ListDefaultChannelTabApps returns apps whose channelTab.enabled AND +// channelTab.default are both true, sorted by channelTab.name asc. +// Projection: _id, avatarUrl, assistant, channelTab. Empty result is ([], nil). +func (s *MongoStore) ListDefaultChannelTabApps(ctx context.Context) ([]model.App, error) { + opts := options.Find(). + SetSort(bson.D{{Key: "channelTab.name", Value: 1}}). + SetProjection(bson.M{ + "_id": 1, + "avatarUrl": 1, + "assistant": 1, + "channelTab": 1, + }) + cursor, err := s.apps.Find(ctx, bson.M{ + "channelTab.enabled": true, + "channelTab.default": true, + }, opts) + if err != nil { + return nil, fmt.Errorf("list default channel-tab apps: %w", err) + } + defer cursor.Close(ctx) + apps := make([]model.App, 0, 8) + if err := cursor.All(ctx, &apps); err != nil { + return nil, fmt.Errorf("decode default channel-tab apps: %w", err) + } + return apps, nil +} + +// ListRoomBotApps returns one entry per bot subscribed to roomID, joined with +// the owning app via assistant.name == u.account. Only apps with +// assistant.enabled=true are emitted. Empty result is ([], nil); result order +// is assistantName asc. +func (s *MongoStore) ListRoomBotApps(ctx context.Context, roomID string) ([]RoomBotAppEntry, error) { + pipeline := mongo.Pipeline{ + {{Key: "$match", Value: bson.M{"roomId": roomID, "u.isBot": true}}}, + {{Key: "$lookup", Value: bson.M{ + "from": "apps", + "let": bson.M{"acct": "$u.account"}, + "pipeline": bson.A{ + bson.M{"$match": bson.M{"$expr": bson.M{"$and": bson.A{ + bson.M{"$eq": bson.A{"$assistant.enabled", true}}, + bson.M{"$eq": bson.A{"$assistant.name", "$$acct"}}, + }}}}, + bson.M{"$project": bson.M{ + "_id": 0, + "assistantName": "$assistant.name", + "appName": "$name", + }}, + }, + "as": "app", + }}}, + {{Key: "$unwind", Value: "$app"}}, + {{Key: "$replaceRoot", Value: bson.M{"newRoot": "$app"}}}, + {{Key: "$sort", Value: bson.D{{Key: "assistantName", Value: 1}}}}, + } + cursor, err := s.subscriptions.Aggregate(ctx, pipeline) + if err != nil { + return nil, fmt.Errorf("list room bot apps for %q: %w", roomID, err) + } + defer cursor.Close(ctx) + entries := make([]RoomBotAppEntry, 0, 4) + if err := cursor.All(ctx, &entries); err != nil { + return nil, fmt.Errorf("decode room bot apps for %q: %w", roomID, err) + } + return entries, nil +} + +// ListActiveCmdMenus returns bot_cmd_menu documents where activeStatus is true +// AND name IN assistantNames, sorted by name asc. Returns ([], nil) when +// assistantNames is empty (skips the query). +func (s *MongoStore) ListActiveCmdMenus(ctx context.Context, assistantNames []string) ([]model.BotCmdMenu, error) { + if len(assistantNames) == 0 { + return []model.BotCmdMenu{}, nil + } + opts := options.Find(). + SetSort(bson.D{{Key: "name", Value: 1}}). + SetProjection(bson.M{ + "_id": 0, + "name": 1, + "cmdBlocks": 1, + }) + cursor, err := s.botCmdMenus.Find(ctx, bson.M{ + "activeStatus": true, + "name": bson.M{"$in": assistantNames}, + }, opts) + if err != nil { + return nil, fmt.Errorf("list active cmd menus: %w", err) + } + defer cursor.Close(ctx) + menus := make([]model.BotCmdMenu, 0, len(assistantNames)) + if err := cursor.All(ctx, &menus); err != nil { + return nil, fmt.Errorf("decode active cmd menus: %w", err) + } + return menus, nil +} + // No order-safety guard on the source-site write; the $lt guard lives on the inbox-worker side. func (s *MongoStore) UpdateThreadSubscriptionRead(ctx context.Context, threadRoomID, account string, lastSeenAt time.Time) error { filter := bson.M{"threadRoomId": threadRoomID, "userAccount": account}