opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-21 20:51:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: scope combined chat source filters (#26137)

Browse Source
This commit is contained in:
Danielle Maywood
2026-06-08 15:05:27 +01:00
committed by GitHub
parent 9db70b6ec8
commit d751b46a19
17 changed files with 167 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+1 -1
View File
@@ -78,7 +78,7 @@ const docTemplate = `{
"parameters": [
{
"type": "string",
"description": "Search query. Supports title:\u003csubstring\u003e (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:\u003cdraft\\|open\\|merged\\|closed\u003e as repeated or comma-separated values, source:\u003ccreated_by_me\\|shared_with_me\\|all\u003e, diff_url:\u003curl\u003e (quote values containing colons), pr:\u003cnumber\u003e (exact PR number match), repo:\u003cowner/repo\u003e (case-insensitive substring match against git remote origin or URL), pr_title:\u003ctext\u003e (case-insensitive PR title substring). Bare terms are not supported; use title:\u003cvalue\u003e for title filtering.",
"description": "Search query. Supports title:\u003csubstring\u003e (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:\u003cdraft\\|open\\|merged\\|closed\u003e as repeated or comma-separated values, source:\u003ccreated_by_me\\|shared_with_me\u003e, diff_url:\u003curl\u003e (quote values containing colons), pr:\u003cnumber\u003e (exact PR number match), repo:\u003cowner/repo\u003e (case-insensitive substring match against git remote origin or URL), pr_title:\u003ctext\u003e (case-insensitive PR title substring). Bare terms are not supported; use title:\u003cvalue\u003e for title filtering.",
"name": "q",
"in": "query"
},
coderd/apidoc/swagger.json
Generated
+1 -1
View File
@@ -59,7 +59,7 @@
"parameters": [
{
"type": "string",
"description": "Search query. Supports title:\u003csubstring\u003e (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:\u003cdraft\\|open\\|merged\\|closed\u003e as repeated or comma-separated values, source:\u003ccreated_by_me\\|shared_with_me\\|all\u003e, diff_url:\u003curl\u003e (quote values containing colons), pr:\u003cnumber\u003e (exact PR number match), repo:\u003cowner/repo\u003e (case-insensitive substring match against git remote origin or URL), pr_title:\u003ctext\u003e (case-insensitive PR title substring). Bare terms are not supported; use title:\u003cvalue\u003e for title filtering.",
"description": "Search query. Supports title:\u003csubstring\u003e (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:\u003cdraft\\|open\\|merged\\|closed\u003e as repeated or comma-separated values, source:\u003ccreated_by_me\\|shared_with_me\u003e, diff_url:\u003curl\u003e (quote values containing colons), pr:\u003cnumber\u003e (exact PR number match), repo:\u003cowner/repo\u003e (case-insensitive substring match against git remote origin or URL), pr_title:\u003ctext\u003e (case-insensitive PR title substring). Bare terms are not supported; use title:\u003cvalue\u003e for title filtering.",
"name": "q",
"in": "query"
},
coderd/database/modelqueries.go
+1 -4
View File
@@ -750,9 +750,6 @@ type chatQuerier interface {
}
func (q *sqlQuerier) GetAuthorizedChats(ctx context.Context, arg GetChatsParams, prepared rbac.PreparedAuthorized) ([]GetChatsRow, error) {
if arg.OwnedOnly && arg.SharedOnly {
return nil, xerrors.New("owned_only and shared_only cannot both be true")
}
if (arg.OwnedOnly || arg.SharedOnly) && arg.ViewerID == uuid.Nil {
return nil, xerrors.New("viewer_id required when owned_only or shared_only is true")
}
@@ -774,8 +771,8 @@ func (q *sqlQuerier) GetAuthorizedChats(ctx context.Context, arg GetChatsParams,
query := fmt.Sprintf("-- name: GetAuthorizedChats :many\n%s", filtered)
rows, err := q.db.QueryContext(ctx, query,
arg.OwnedOnly,
arg.ViewerID,
arg.SharedOnly,
arg.ViewerID,
arg.SharedWithUserID,
pq.Array(arg.SharedWithGroupIds),
arg.Archived,
coderd/database/querier_test.go
+7 -5
View File
@@ -1587,12 +1587,14 @@ func TestGetAuthorizedChatsACLSharing(t *testing.T) {
require.Equal(t, sharedACL, sharedOnly[0].Chat.UserACL)
require.Empty(t, sharedOnly[0].Chat.GroupACL)
_, err = db.GetAuthorizedChats(ctx, database.GetChatsParams{
OwnedOnly: true,
SharedOnly: true,
ViewerID: recipient.ID,
ownedAndShared, err := db.GetAuthorizedChats(ctx, database.GetChatsParams{
OwnedOnly: true,
SharedOnly: true,
ViewerID: recipient.ID,
SharedWithUserID: recipient.ID,
}, preparedRecipient)
require.ErrorContains(t, err, "owned_only and shared_only")
require.NoError(t, err)
require.ElementsMatch(t, []uuid.UUID{ownerChat.ID, recipientChat.ID}, chatIDs(ownedAndShared))
authzdb := dbauthz.New(db, authorizer, slogtest.Make(t, &slogtest.Options{}), coderdtest.AccessControlStorePointer())
recipientCtx := dbauthz.As(ctx, recipientSubject)
coderd/database/queries.sql.go
Generated
+10 -11
View File
@@ -8023,19 +8023,18 @@ SELECT
FROM
chats_expanded
WHERE
CASE
WHEN $1::boolean THEN chats_expanded.owner_id = $2::uuid
ELSE true
END
AND CASE
WHEN $3::boolean THEN
chats_expanded.owner_id != $2::uuid
(
(NOT $1::boolean AND NOT $2::boolean)
OR ($1::boolean AND chats_expanded.owner_id = $3::uuid)
OR (
$2::boolean
AND chats_expanded.owner_id != $3::uuid
AND (
chats_expanded.user_acl ? ($4::uuid)::text
OR chats_expanded.group_acl ?| $5::text[]
)
ELSE true
END
)
)
AND CASE
WHEN $6 :: boolean IS NULL THEN true
ELSE chats_expanded.archived = $6 :: boolean
@@ -8174,8 +8173,8 @@ LIMIT
type GetChatsParams struct {
OwnedOnly bool `db:"owned_only" json:"owned_only"`
ViewerID uuid.UUID `db:"viewer_id" json:"viewer_id"`
SharedOnly bool `db:"shared_only" json:"shared_only"`
ViewerID uuid.UUID `db:"viewer_id" json:"viewer_id"`
SharedWithUserID uuid.UUID `db:"shared_with_user_id" json:"shared_with_user_id"`
SharedWithGroupIds []string `db:"shared_with_group_ids" json:"shared_with_group_ids"`
Archived sql.NullBool `db:"archived" json:"archived"`
@@ -8200,8 +8199,8 @@ type GetChatsRow struct {
func (q *sqlQuerier) GetChats(ctx context.Context, arg GetChatsParams) ([]GetChatsRow, error) {
rows, err := q.db.QueryContext(ctx, getChats,
arg.OwnedOnly,
arg.ViewerID,
arg.SharedOnly,
arg.ViewerID,
arg.SharedWithUserID,
pq.Array(arg.SharedWithGroupIds),
arg.Archived,
coderd/database/queries/chats.sql
+8 -9
View File
@@ -481,19 +481,18 @@ SELECT
FROM
chats_expanded
WHERE
CASE
WHEN @owned_only::boolean THEN chats_expanded.owner_id = @viewer_id::uuid
ELSE true
END
AND CASE
WHEN @shared_only::boolean THEN
chats_expanded.owner_id != @viewer_id::uuid
(
(NOT @owned_only::boolean AND NOT @shared_only::boolean)
OR (@owned_only::boolean AND chats_expanded.owner_id = @viewer_id::uuid)
OR (
@shared_only::boolean
AND chats_expanded.owner_id != @viewer_id::uuid
AND (
chats_expanded.user_acl ? (@shared_with_user_id::uuid)::text
OR chats_expanded.group_acl ?| @shared_with_group_ids::text[]
)
ELSE true
END
)
)
AND CASE
WHEN sqlc.narg('archived') :: boolean IS NULL THEN true
ELSE chats_expanded.archived = sqlc.narg('archived') :: boolean
coderd/exp_chats.go
+1 -1
View File
@@ -339,7 +339,7 @@ func (api *API) chatsByWorkspace(rw http.ResponseWriter, r *http.Request) {
// @Security CoderSessionToken
// @Tags Chats
// @Produce json
// @Param q query string false "Search query. Supports title:<substring> (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:<draft\|open\|merged\|closed> as repeated or comma-separated values, source:<created_by_me\|shared_with_me\|all>, diff_url:<url> (quote values containing colons), pr:<number> (exact PR number match), repo:<owner/repo> (case-insensitive substring match against git remote origin or URL), pr_title:<text> (case-insensitive PR title substring). Bare terms are not supported; use title:<value> for title filtering."
// @Param q query string false "Search query. Supports title:<substring> (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:<draft\|open\|merged\|closed> as repeated or comma-separated values, source:<created_by_me\|shared_with_me>, diff_url:<url> (quote values containing colons), pr:<number> (exact PR number match), repo:<owner/repo> (case-insensitive substring match against git remote origin or URL), pr_title:<text> (case-insensitive PR title substring). Bare terms are not supported; use title:<value> for title filtering."
// @Param label query string false "Filter by label as key:value. Repeat for multiple (AND logic)."
// @Success 200 {array} codersdk.Chat
// @Router /api/experimental/chats [get]
coderd/exp_chats_acl_test.go
+2 -2
View File
@@ -432,9 +432,9 @@ func TestListChatsSharedScope(t *testing.T) {
shared: map[uuid.UUID]bool{sharedChat.ID: true},
},
{
name: "all",
name: "created by me and shared with me",
opts: &codersdk.ListChatsOptions{
Source: codersdk.ChatListSourceAll,
Query: "source:created_by_me,shared_with_me",
},
expected: map[uuid.UUID]struct{}{viewerChat.ID: {}, sharedChat.ID: {}},
shared: map[uuid.UUID]bool{viewerChat.ID: false, sharedChat.ID: true},
coderd/exp_chats_test.go
+85
View File
@@ -1002,6 +1002,91 @@ func TestListChats(t *testing.T) {
require.Equal(t, memberChats[0].ID, memberChats[0].DiffStatus.ChatID)
})
t.Run("SourceCreatedByMeAndSharedWithMeExcludesUnsharedReadableChats", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, db := newChatClientWithDatabase(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
modelConfig := createChatModelConfig(t, client)
ownerClientRaw, owner := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.RoleOwner())
ownerClient := codersdk.NewExperimentalClient(ownerClientRaw)
memberClientRaw, member := coderdtest.CreateAnotherUser(t, client.Client, firstUser.OrganizationID, rbac.ScopedRoleAgentsAccess(firstUser.OrganizationID))
memberClient := codersdk.NewExperimentalClient(memberClientRaw)
ownedChat := dbgen.Chat(t, db, database.Chat{
OrganizationID: firstUser.OrganizationID,
OwnerID: owner.ID,
LastModelConfigID: modelConfig.ID,
Title: "owner created chat",
Status: database.ChatStatusCompleted,
})
sharedChat := dbgen.Chat(t, db, database.Chat{
OrganizationID: firstUser.OrganizationID,
OwnerID: member.ID,
LastModelConfigID: modelConfig.ID,
Title: "member shared chat",
Status: database.ChatStatusCompleted,
})
unsharedReadableChat := dbgen.Chat(t, db, database.Chat{
OrganizationID: firstUser.OrganizationID,
OwnerID: firstUser.UserID,
LastModelConfigID: modelConfig.ID,
Title: "unshared readable chat",
Status: database.ChatStatusCompleted,
})
err := db.UpdateChatACLByID(dbauthz.As(ctx, rbac.Subject{
ID: member.ID.String(),
Roles: rbac.RoleIdentifiers{rbac.RoleOwner()},
Scope: rbac.ScopeAll,
}), database.UpdateChatACLByIDParams{
ID: sharedChat.ID,
UserACL: database.ChatACL{
owner.ID.String(): database.ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}},
},
GroupACL: database.ChatACL{},
})
require.NoError(t, err)
ownerChats, err := ownerClient.ListChats(ctx, &codersdk.ListChatsOptions{
Query: "source:created_by_me,shared_with_me",
})
require.NoError(t, err)
ownerChatIDs := make(map[uuid.UUID]struct{}, len(ownerChats))
for _, chat := range ownerChats {
ownerChatIDs[chat.ID] = struct{}{}
}
require.Contains(t, ownerChatIDs, ownedChat.ID)
require.Contains(t, ownerChatIDs, sharedChat.ID)
require.NotContains(t, ownerChatIDs, unsharedReadableChat.ID)
sharedOnlyChats, err := ownerClient.ListChats(ctx, &codersdk.ListChatsOptions{
Source: codersdk.ChatListSourceSharedWithMe,
})
require.NoError(t, err)
sharedOnlyChatIDs := make(map[uuid.UUID]struct{}, len(sharedOnlyChats))
for _, chat := range sharedOnlyChats {
sharedOnlyChatIDs[chat.ID] = struct{}{}
}
require.Contains(t, sharedOnlyChatIDs, sharedChat.ID)
require.NotContains(t, sharedOnlyChatIDs, ownedChat.ID)
require.NotContains(t, sharedOnlyChatIDs, unsharedReadableChat.ID)
memberChats, err := memberClient.ListChats(ctx, &codersdk.ListChatsOptions{
Query: "source:created_by_me,shared_with_me",
})
require.NoError(t, err)
memberChatIDs := make(map[uuid.UUID]struct{}, len(memberChats))
for _, chat := range memberChats {
memberChatIDs[chat.ID] = struct{}{}
}
require.Contains(t, memberChatIDs, sharedChat.ID)
require.NotContains(t, memberChatIDs, ownedChat.ID)
require.NotContains(t, memberChatIDs, unsharedReadableChat.ID)
})
t.Run("OrgMemberWithoutAgentsAccessCannotAccessOwnChats", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
coderd/searchquery/search.go
+19 -11
View File
@@ -5,6 +5,7 @@ import (
"database/sql"
"fmt"
"net/url"
"slices"
"strconv"
"strings"
"time"
@@ -611,22 +612,29 @@ func Chats(query string) (database.GetChatsParams, []codersdk.ValidationError) {
filter.TitleQuery = parser.String(values, "", "title")
filter.PrTitleQuery = parser.String(values, "", "pr_title")
filter.RepoQuery = parser.String(values, "", "repo")
if source := parser.String(values, "", "source"); source != "" {
sources := httpapi.ParseCustomList(parser, values, nil, "source", func(v string) (string, error) {
source := strings.ToLower(strings.TrimSpace(v))
switch source {
case "created_by_me":
case "created_by_me", "shared_with_me":
return source, nil
default:
return "", xerrors.Errorf("%q is not a valid value", v)
}
})
if len(sources) > 0 {
hasCreatedByMe := slices.Contains(sources, "created_by_me")
hasSharedWithMe := slices.Contains(sources, "shared_with_me")
switch {
case hasCreatedByMe && hasSharedWithMe:
filter.OwnedOnly = true
filter.SharedOnly = false
case "shared_with_me":
filter.SharedOnly = true
case hasSharedWithMe:
filter.OwnedOnly = false
filter.SharedOnly = true
case "all":
filter.OwnedOnly = false
filter.SharedOnly = false
default:
parser.Errors = append(parser.Errors, codersdk.ValidationError{
Field: "source",
Detail: fmt.Sprintf("%q is not a valid value", source),
})
filter.OwnedOnly = true
filter.SharedOnly = false
}
}
coderd/searchquery/search_test.go
+19 -8
View File
@@ -1378,11 +1378,9 @@ func TestSearchChats(t *testing.T) {
},
},
{
Name: "SourceAll",
Query: "source:all",
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
},
Name: "SourceAllInvalid",
Query: "source:all",
ExpectedErrorContains: "source",
},
{
Name: "SourceInvalid",
@@ -1390,9 +1388,22 @@ func TestSearchChats(t *testing.T) {
ExpectedErrorContains: "source",
},
{
Name: "SourceRepeated",
Query: "source:created_by_me source:shared_with_me",
ExpectedErrorContains: "source",
Name: "SourceCreatedByMeAndSharedWithMe",
Query: "source:created_by_me,shared_with_me",
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
OwnedOnly: true,
SharedOnly: true,
},
},
{
Name: "SourceRepeated",
Query: "source:created_by_me source:shared_with_me",
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
OwnedOnly: true,
SharedOnly: true,
},
},
{
Name: "ExtraParam",
codersdk/chats.go
-2
View File
@@ -2047,8 +2047,6 @@ const (
ChatListSourceCreatedByMe ChatListSource = "created_by_me"
// ChatListSourceSharedWithMe returns chats shared with the caller.
ChatListSourceSharedWithMe ChatListSource = "shared_with_me"
// ChatListSourceAll returns both owned and shared chats.
ChatListSourceAll ChatListSource = "all"
)
// ListChatsOptions are optional parameters for ListChats.
docs/reference/api/chats.md
Generated
+4 -4
View File
@@ -17,10 +17,10 @@ Experimental: this endpoint is subject to change.
### Parameters
| Name | In | Type | Required | Description |
|---------|-------|--------|----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `q` | query | string | false | Search query. Supports title:<substring> (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:<draft\|open\|merged\|closed> as repeated or comma-separated values, source:<created_by_me\|shared_with_me\|all>, diff_url:<url> (quote values containing colons), pr:<number> (exact PR number match), repo:<owner/repo> (case-insensitive substring match against git remote origin or URL), pr_title:<text> (case-insensitive PR title substring). Bare terms are not supported; use title:<value> for title filtering. |
| `label` | query | string | false | Filter by label as key:value. Repeat for multiple (AND logic). |
| Name | In | Type | Required | Description |
|---------|-------|--------|----------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `q` | query | string | false | Search query. Supports title:<substring> (case-insensitive, quote multi-word values), archived:bool, has_unread:bool, pr_status:<draft\|open\|merged\|closed> as repeated or comma-separated values, source:<created_by_me\|shared_with_me>, diff_url:<url> (quote values containing colons), pr:<number> (exact PR number match), repo:<owner/repo> (case-insensitive substring match against git remote origin or URL), pr_title:<text> (case-insensitive PR title substring). Bare terms are not supported; use title:<value> for title filtering. |
| `label` | query | string | false | Filter by label as key:value. Repeat for multiple (AND logic). |
### Example responses
site/src/api/queries/chats.test.ts
+3 -3
View File
@@ -1543,13 +1543,13 @@ describe("infiniteChats", () => {
});
});
it("builds q from archived, prStatuses, chatStatus, and source", async () => {
it("builds q from archived, prStatuses, chatStatus, and sources", async () => {
vi.mocked(API.experimental.getChats).mockResolvedValue([]);
const { queryFn } = infiniteChats({
archived: true,
prStatuses: ["draft", "open", "merged"],
chatStatus: "unread",
source: "all",
sources: ["created_by_me", "shared_with_me"],
});
await queryFn({ pageParam: 0 });
@@ -1557,7 +1557,7 @@ describe("infiniteChats", () => {
expect(API.experimental.getChats).toHaveBeenCalledWith({
limit: PAGE_LIMIT,
offset: 0,
q: "archived:true pr_status:draft,open,merged has_unread:true source:all",
q: "archived:true pr_status:draft,open,merged has_unread:true source:created_by_me,shared_with_me",
});
});
site/src/api/queries/chats.ts
+3 -3
View File
@@ -34,7 +34,7 @@ type InfiniteChatsFilters = Readonly<{
archived?: boolean;
prStatuses?: readonly ChatListPRStatusFilter[];
chatStatus?: ChatListStatusFilter;
source?: TypesGen.ChatListSource;
sources?: readonly TypesGen.ChatListSource[];
}>;
export const infiniteChatsKey = (filters?: InfiniteChatsFilters) =>
@@ -559,8 +559,8 @@ const getInfiniteChatsQueryString = (
if (filters?.chatStatus) {
qParts.push(`has_unread:${filters.chatStatus === "unread"}`);
}
if (filters?.source) {
qParts.push(`source:${filters.source}`);
if (filters?.sources?.length) {
qParts.push(`source:${filters.sources.join(",")}`);
}
return qParts.length > 0 ? qParts.join(" ") : undefined;
};
site/src/api/typesGenerated.ts
Generated
+1 -2
View File
@@ -2151,10 +2151,9 @@ export const ChatInputPartTypes: ChatInputPartType[] = [
];
// From codersdk/chats.go
export type ChatListSource = "all" | "created_by_me" | "shared_with_me";
export type ChatListSource = "created_by_me" | "shared_with_me";
export const ChatListSources: ChatListSource[] = [
"all",
"created_by_me",
"shared_with_me",
];
site/src/pages/AgentsPage/AgentsPage.tsx
+2 -9
View File
@@ -56,10 +56,7 @@ import { AgentsPageView } from "./AgentsPageView";
import { emptyInputStorageKey } from "./components/AgentCreateForm";
import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings";
import { useAgentsPWA } from "./hooks/useAgentsPWA";
import {
AGENT_SOURCE_ORDER,
getAgentSidebarFilters,
} from "./utils/agentSidebarFilters";
import { getAgentSidebarFilters } from "./utils/agentSidebarFilters";
import {
archiveChatAndDeleteWorkspace,
resolveArchiveAndDeleteAction,
@@ -152,16 +149,12 @@ const AgentsPage: FC = () => {
sidebarFilters.chatStatuses.length === 1
? sidebarFilters.chatStatuses[0]
: undefined;
const sourceFilter =
sidebarFilters.sources.length === AGENT_SOURCE_ORDER.length
? "all"
: sidebarFilters.sources[0];
const chatsQuery = useInfiniteQuery(
infiniteChats({
archived: archivedFilter,
prStatuses: sidebarFilters.prStatuses,
chatStatus: chatStatusFilter,
source: sourceFilter,
sources: sidebarFilters.sources,
}),
);
// Model queries are kept here for the sidebar, which displays