From 14eeb23a4b7b056a7127b0cbbbfc7bf9c66bbd0b Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Fri, 11 Sep 2026 17:01:27 +0000 Subject: [PATCH 1/3] feat: share a new chat with its organization on request Chats are owned by the coder-token holder, so the chat-url the action posts answers "Chat not found" for everyone else. share-with-organization grants the chat's organization read access on creation, off by default. --- AGENTS.md | 3 ++ README.md | 16 +++++++ action.yaml | 5 +++ dist/index.js | 31 +++++++++++++- scripts/typegen/main.go | 1 + src/action.test.ts | 93 ++++++++++++++++++++++++++++++++++++++++ src/action.ts | 39 +++++++++++++++++ src/coder-client.test.ts | 34 +++++++++++++++ src/coder-client.ts | 25 +++++++++++ src/codersdk.gen.ts | 11 +++++ src/index.ts | 1 + src/schemas.test.ts | 8 ++++ src/schemas.ts | 1 + src/test-helpers.ts | 9 ++++ 14 files changed, 275 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0c0bb9b..2b2b41c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,3 +101,6 @@ bun run build - `POST /api/experimental/chats/{id}/messages` - Send message - `GET /api/experimental/chats/{id}` - Get chat - `GET /api/experimental/chats` - List chats + +- **Chat sharing**: + - `PATCH /api/v2/chats/{id}/acl` - Grant read access to users or groups (used by `share-with-organization`) diff --git a/README.md b/README.md index 84d812e..3d012bf 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ The chat runs as whoever the `coder-token` belongs to; that identity is the only | `wait-timeout-seconds` | no | `600` | Max wait when `wait: complete`. | | `idempotency-key` | no | | Optional sharding key on the reuse scope. See [Chat reuse](#chat-reuse). | | `force-new-chat` | no | `false` | Skip chat-reuse lookup and always create. Mutually exclusive with `existing-chat-id`. | +| `share-with-organization` | no | `false` | Give the chat's Coder organization read access to a newly created chat. See [Who can read the chat](#who-can-read-the-chat). | ## Outputs @@ -112,6 +113,21 @@ There is one Coder identity in play. `POST /api/experimental/chats` binds the ch Either path fails with `chat-error-kind=org_not_found` when the org doesn't exist or the user has no memberships. +### Who can read the chat + +Every chat is owned by the `coder-token` holder, usually a bot account. By default nobody else can open it, and the `chat-url` in the issue comment answers "Chat not found" for everyone but that bot. That hides the agent's reasoning from the people reading its output. + +Set `share-with-organization: true` to grant the chat's organization read access. The Everyone group shares its organization's ID, so one group entry covers every member of the organization the chat runs in, resolved the same way as [Organization resolution](#organization-resolution) above. + +Details worth knowing: + +- Read-only. Readers can open the chat and follow it; they cannot send messages. +- Only on creation. A reused chat keeps the access it already had, so turning the input on does not retroactively open past chats. +- Applied before the `wait: complete` poll, so a reader can watch a long run rather than only read it afterwards. +- Group access sends no notifications. Sharing a chat with a named user does. +- Sub-chats are separate. Coder sets ACLs on root chats only, so a subagent's chat is not covered by the parent's entry. +- A deployment with chat sharing disabled answers `403` here. The action logs a warning and the run still succeeds, because the chat itself is fine. + ### Chat reuse By default the action reuses the most recent non-archived chat scoped to the same `github-url` and (when `GITHUB_WORKFLOW` is set) the same workflow name. Two workflows targeting the same PR keep separate chats. Re-running the same workflow continues one chat. diff --git a/action.yaml b/action.yaml index e62c26e..0c8ddb7 100644 --- a/action.yaml +++ b/action.yaml @@ -67,6 +67,11 @@ inputs: required: false default: "false" + share-with-organization: + description: "Give the chat's Coder organization read access to a newly created chat, so people other than the `coder-token` holder can open it. Only applies when this run creates the chat; a reused chat keeps whatever access it already had. Read-only, and group access sends no notifications. A deployment with chat sharing disabled logs a warning instead of failing the run." + required: false + default: "false" + outputs: coder-username: description: "The Coder username the `coder-token` belongs to (always the chat owner; the chats API has no owner override)." diff --git a/dist/index.js b/dist/index.js index d85efbb..4034c18 100644 --- a/dist/index.js +++ b/dist/index.js @@ -36555,6 +36555,7 @@ var ChatInputPartSchema = exports_external.object({ content: exports_external.string().optional() }); var ChatPlanModeSchema = exports_external.enum(["plan"]); +var ChatRoleSchema = exports_external.enum(["", "read"]); var ChatStatusSchema = exports_external.enum([ "completed", "error", @@ -36650,6 +36651,10 @@ var SlimRoleSchema = exports_external.object({ display_name: exports_external.string(), organization_id: exports_external.string().optional() }); +var UpdateChatACLSchema = exports_external.object({ + user_roles: exports_external.record(exports_external.string(), ChatRoleSchema).optional(), + group_roles: exports_external.record(exports_external.string(), ChatRoleSchema).optional() +}); var UserStatusSchema = exports_external.enum(["active", "dormant", "suspended"]); var ReducedUserSchema = MinimalUserSchema.extend({ email: exports_external.string(), @@ -36745,6 +36750,13 @@ class RealCoderClient { const response = await this.request(endpoint2); return CoderChatSchema.parse(response); } + async updateChatACL(chatId, params) { + const endpoint2 = `/api/v2/chats/${encodeURIComponent(chatId)}/acl`; + await this.request(endpoint2, { + method: "PATCH", + body: JSON.stringify(params) + }); + } async listChats(opts) { const params = []; if (opts?.label !== undefined) { @@ -37374,6 +37386,9 @@ class CoderAgentChatAction { info(`Agents chat created successfully (id: ${createdChat.id}, status: ${createdChat.status})`); const chatUrl = this.generateChatUrl(createdChat.id); info(`Chat URL: ${chatUrl}`); + if (this.inputs.shareWithOrganization) { + await this.shareWithOrganization(createdChat.id, organizationID); + } let finalChat = createdChat; if (this.inputs.wait === "complete") { info(`Waiting for chat to reach terminal status (timeout: ${this.inputs.waitTimeoutSeconds}s)...`); @@ -37399,6 +37414,16 @@ class CoderAgentChatAction { } return this.buildOutputs(coderUsername, finalChat, true); } + async shareWithOrganization(chatId, organizationID) { + try { + await this.coder.updateChatACL(chatId, { + group_roles: { [organizationID]: "read" } + }); + info(`Granted read access on the chat to organization ${organizationID}`); + } catch (error52) { + warning(`Could not share the chat with organization ${organizationID}: ${error52 instanceof Error ? error52.message : String(error52)}`); + } + } async runFollowUp(args) { const { coderUsername, @@ -37566,7 +37591,8 @@ var ActionInputsObjectSchema = exports_external.object({ wait: exports_external.enum(["none", "complete"]).default("none"), waitTimeoutSeconds: exports_external.coerce.number().int().positive().default(DEFAULT_WAIT_TIMEOUT_SECONDS), idempotencyKey: exports_external.string().min(1).optional(), - forceNewChat: exports_external.boolean().default(false) + forceNewChat: exports_external.boolean().default(false), + shareWithOrganization: exports_external.boolean().default(false) }); var ActionInputsSchema = ActionInputsObjectSchema.refine((data) => !(data.existingChatId !== undefined && data.forceNewChat === true), { message: "Cannot set both existing-chat-id and force-new-chat; choose one.", @@ -37616,7 +37642,8 @@ async function main() { wait: getInput("wait") || undefined, waitTimeoutSeconds: getInput("wait-timeout-seconds") || undefined, idempotencyKey: getInput("idempotency-key") || undefined, - forceNewChat: getBooleanInput("force-new-chat") + forceNewChat: getBooleanInput("force-new-chat"), + shareWithOrganization: getBooleanInput("share-with-organization") }); debug("Inputs validated successfully"); debug(`Coder URL: ${inputs.coderURL}`); diff --git a/scripts/typegen/main.go b/scripts/typegen/main.go index c5583f9..edadd36 100644 --- a/scripts/typegen/main.go +++ b/scripts/typegen/main.go @@ -27,6 +27,7 @@ var wantedTypes = map[string]bool{ "Chat": true, "CreateChatMessageRequest": true, "CreateChatRequest": true, + "UpdateChatACL": true, "Organization": true, "User": true, } diff --git a/src/action.test.ts b/src/action.test.ts index 3635bac..705371d 100644 --- a/src/action.test.ts +++ b/src/action.test.ts @@ -2260,6 +2260,99 @@ describe("CoderAgentChatAction", () => { }); }); + describe("share-with-organization", () => { + test("grants the resolved organization read access on a new chat", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockCreateChat.mockResolvedValue(mockChat); + + const inputs = createMockInputs({ + coderOrganization: "coder", + shareWithOrganization: true, + }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + await action.run(); + + // The Everyone group shares its organization's ID, so the entry + // is keyed by the same UUID that createChat received. + expect(coderClient.mockUpdateChatACL).toHaveBeenCalledWith(mockChat.id, { + group_roles: { [mockOrganization.id]: "read" }, + }); + }); + + test("shares nothing by default", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockCreateChat.mockResolvedValue(mockChat); + + const inputs = createMockInputs({}); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + await action.run(); + + expect(coderClient.mockUpdateChatACL).not.toHaveBeenCalled(); + }); + + test("does not re-share a reused chat", async () => { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockListChats.mockResolvedValue([mockChat]); + coderClient.mockCreateChatMessage.mockResolvedValue( + mockChatMessageResponse, + ); + coderClient.mockGetChat.mockResolvedValue(mockChat); + + const inputs = createMockInputs({ shareWithOrganization: true }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + await action.run(); + + expect(coderClient.mockCreateChat).not.toHaveBeenCalled(); + expect(coderClient.mockUpdateChatACL).not.toHaveBeenCalled(); + }); + + test("warns and still succeeds when sharing fails", async () => { + const warning = spyOn(core, "warning").mockImplementation(() => {}); + try { + coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); + coderClient.mockCreateChat.mockResolvedValue(mockChat); + coderClient.mockUpdateChatACL.mockRejectedValue( + new CoderAPIError( + "Chat sharing is disabled for this deployment.", + 403, + ), + ); + + const inputs = createMockInputs({ shareWithOrganization: true }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + const outputs = await action.run(); + + expect(outputs.chatId).toBe(mockChat.id); + expect(outputs.chatCreated).toBe(true); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("Could not share the chat"), + ); + } finally { + warning.mockRestore(); + } + }); + }); + describe("Chat reuse", () => { test("default: listChats is called with the gh-target scope before creating", async () => { coderClient.mockGetAuthenticatedUser.mockResolvedValue(mockUser); diff --git a/src/action.ts b/src/action.ts index 810577c..c420f93 100644 --- a/src/action.ts +++ b/src/action.ts @@ -647,6 +647,12 @@ export class CoderAgentChatAction { const chatUrl = this.generateChatUrl(createdChat.id); core.info(`Chat URL: ${chatUrl}`); + // Share before polling, so the chat is readable while it runs + // rather than only after it finishes. + if (this.inputs.shareWithOrganization) { + await this.shareWithOrganization(createdChat.id, organizationID); + } + // Poll before commenting so wait=complete posts only after the // chat reaches a terminal state. No mid-poll comment updates. let finalChat = createdChat; @@ -681,6 +687,39 @@ export class CoderAgentChatAction { return this.buildOutputs(coderUsername, finalChat, true); } + /** + * Give the chat's organization read access, so anyone in it can open + * the chat this run created. The Everyone group shares its + * organization's ID, so a single group entry covers every member. + * + * Only the create branch calls this. A reused chat was shared by the + * run that created it, and a chat created before this input existed + * stays private on purpose: re-sharing it would be a silent change to + * who can read past work. + * + * A sharing failure never fails the run. The review itself is fine, + * and a deployment with chat sharing disabled answers 403 here. + */ + private async shareWithOrganization( + chatId: ChatId, + organizationID: string, + ): Promise { + try { + await this.coder.updateChatACL(chatId, { + group_roles: { [organizationID]: "read" }, + }); + core.info( + `Granted read access on the chat to organization ${organizationID}`, + ); + } catch (error) { + core.warning( + `Could not share the chat with organization ${organizationID}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + /** * Send `chat-prompt` as a follow-up message to an existing chat and * complete the post-message flow (poll under `wait: complete`, refresh diff --git a/src/coder-client.test.ts b/src/coder-client.test.ts index 7bed088..2c60f6f 100644 --- a/src/coder-client.test.ts +++ b/src/coder-client.test.ts @@ -137,6 +137,40 @@ describe("CoderClient", () => { }); }); + describe("updateChatACL", () => { + test("patches the v2 ACL route with the group role", async () => { + mockFetch.mockResolvedValue( + createMockResponse(undefined, { status: 204 }), + ); + await client.updateChatACL(mockChat.id, { + group_roles: { "cc0e8400-e29b-41d4-a716-446655440000": "read" }, + }); + expect(mockFetch).toHaveBeenCalledWith( + `https://coder.test/api/v2/chats/${mockChat.id}/acl`, + expect.objectContaining({ + method: "PATCH", + body: JSON.stringify({ + group_roles: { "cc0e8400-e29b-41d4-a716-446655440000": "read" }, + }), + }), + ); + }); + + test("throws when sharing is disabled for the deployment", async () => { + mockFetch.mockResolvedValue( + createMockResponse( + { message: "Chat sharing is disabled for this deployment." }, + { ok: false, status: 403, statusText: "Forbidden" }, + ), + ); + expect( + client.updateChatACL(mockChat.id, { + group_roles: { "cc0e8400-e29b-41d4-a716-446655440000": "read" }, + }), + ).rejects.toThrow(CoderAPIError); + }); + }); + describe("getAuthenticatedUser", () => { test("returns the user behind the configured token", async () => { mockFetch.mockResolvedValueOnce(createMockResponse(mockUser)); diff --git a/src/coder-client.ts b/src/coder-client.ts index 370275a..72c9ab0 100644 --- a/src/coder-client.ts +++ b/src/coder-client.ts @@ -4,16 +4,19 @@ import { ChatSchema, ChatDiffStatusSchema, ChatErrorSchema, + ChatRoleSchema, ChatStatusSchema, CreateChatMessageRequestSchema, CreateChatRequestSchema, OrganizationSchema, + UpdateChatACLSchema, UserSchema, } from "./codersdk.gen"; import type { CreateChatMessageRequest, CreateChatRequest, Organization, + UpdateChatACL, User, } from "./codersdk.gen"; @@ -31,20 +34,24 @@ export { ChatSchema, ChatDiffStatusSchema, ChatErrorSchema, + ChatRoleSchema, ChatStatusSchema, CreateChatMessageRequestSchema, CreateChatRequestSchema, OrganizationSchema, + UpdateChatACLSchema, UserSchema, }; export type { Chat, ChatDiffStatus, ChatError, + ChatRole, ChatStatus, CreateChatMessageRequest, CreateChatRequest, Organization, + UpdateChatACL, User, } from "./codersdk.gen"; @@ -87,6 +94,14 @@ export interface CoderClient { getChat(chatId: ChatId): Promise; listChats(opts?: ListChatsOptions): Promise; + + /** + * Grant read access on a chat to users or groups via + * `PATCH /api/v2/chats/{chat}/acl`. Every chat is owned by the + * `coder-token` holder, so without an ACL entry nobody else can open + * one. Group entries send no notifications; user entries do. + */ + updateChatACL(chatId: ChatId, params: UpdateChatACL): Promise; } export interface ListChatsOptions { @@ -202,6 +217,16 @@ export class RealCoderClient implements CoderClient { return CoderChatSchema.parse(response); } + async updateChatACL(chatId: ChatId, params: UpdateChatACL): Promise { + // The ACL route is served from /api/v2. The chat routes above still + // use the /api/experimental mount, which is the older prefix. + const endpoint = `/api/v2/chats/${encodeURIComponent(chatId)}/acl`; + await this.request(endpoint, { + method: "PATCH", + body: JSON.stringify(params), + }); + } + async listChats(opts?: ListChatsOptions): Promise { const params: string[] = []; if (opts?.label !== undefined) { diff --git a/src/codersdk.gen.ts b/src/codersdk.gen.ts index 677f5e8..e1ea95b 100644 --- a/src/codersdk.gen.ts +++ b/src/codersdk.gen.ts @@ -93,6 +93,10 @@ export const ChatPlanModeSchema = z.enum(["plan"]); export type ChatPlanMode = z.infer; +export const ChatRoleSchema = z.enum(["", "read"]); + +export type ChatRole = z.infer; + export const ChatStatusSchema = z.enum([ "completed", "error", @@ -220,6 +224,13 @@ export const SlimRoleSchema = z.object({ export type SlimRole = z.infer; +export const UpdateChatACLSchema = z.object({ + user_roles: z.record(z.string(), ChatRoleSchema).optional(), + group_roles: z.record(z.string(), ChatRoleSchema).optional(), +}); + +export type UpdateChatACL = z.infer; + export const UserStatusSchema = z.enum(["active", "dormant", "suspended"]); export type UserStatus = z.infer; diff --git a/src/index.ts b/src/index.ts index bd38911..db085fc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,6 +22,7 @@ async function main() { waitTimeoutSeconds: core.getInput("wait-timeout-seconds") || undefined, idempotencyKey: core.getInput("idempotency-key") || undefined, forceNewChat: core.getBooleanInput("force-new-chat"), + shareWithOrganization: core.getBooleanInput("share-with-organization"), }); core.debug("Inputs validated successfully"); diff --git a/src/schemas.test.ts b/src/schemas.test.ts index 6a80238..335dc20 100644 --- a/src/schemas.test.ts +++ b/src/schemas.test.ts @@ -19,6 +19,7 @@ const actionInputValid: ActionInputs = { wait: "none", waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, forceNewChat: false, + shareWithOrganization: false, }; describe("ActionInputsSchema", () => { @@ -95,6 +96,13 @@ describe("ActionInputsSchema", () => { expect(result.coderURL).toBe(url); } }); + + test("shareWithOrganization defaults to false when omitted", () => { + const { shareWithOrganization, ...withoutShare } = actionInputValid; + expect(shareWithOrganization).toBe(false); + const result = ActionInputsSchema.parse(withoutShare); + expect(result.shareWithOrganization).toBe(false); + }); }); describe("Invalid Input Cases", () => { diff --git a/src/schemas.ts b/src/schemas.ts index 593606c..75d890b 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -23,6 +23,7 @@ const ActionInputsObjectSchema = z.object({ .default(DEFAULT_WAIT_TIMEOUT_SECONDS), idempotencyKey: z.string().min(1).optional(), forceNewChat: z.boolean().default(false), + shareWithOrganization: z.boolean().default(false), }); export const ActionInputsSchema = ActionInputsObjectSchema.refine( diff --git a/src/test-helpers.ts b/src/test-helpers.ts index 25aa0a6..f2820ac 100644 --- a/src/test-helpers.ts +++ b/src/test-helpers.ts @@ -12,6 +12,7 @@ import type { CreateChatMessageRequest, CreateChatMessageResponse, ChatId, + UpdateChatACL, } from "./coder-client"; import type { Clock } from "./action"; import type { ActionInputs } from "./schemas"; @@ -143,6 +144,7 @@ export function createMockInputs( wait: "none", waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, forceNewChat: false, + shareWithOrganization: false, ...overrides, } as ActionInputs; } @@ -161,6 +163,9 @@ export class MockCoderClient implements CoderClient { Promise.resolve([] as CoderChat[]), ); public mockGetAuthenticatedUser = mock(() => Promise.resolve(mockUser)); + public mockUpdateChatACL = mock((_chatId: ChatId, _params: UpdateChatACL) => + Promise.resolve(), + ); async getAuthenticatedUser(): Promise { return this.mockGetAuthenticatedUser(); @@ -188,6 +193,10 @@ export class MockCoderClient implements CoderClient { async listChats(opts?: ListChatsOptions): Promise { return this.mockListChats(opts); } + + async updateChatACL(chatId: ChatId, params: UpdateChatACL): Promise { + return this.mockUpdateChatACL(chatId, params); + } } /** From 5169089efd712bfdeaccee7fa0341c15df1d2abd Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Fri, 11 Sep 2026 17:09:58 +0000 Subject: [PATCH 2/3] feat: share a new chat with groups and users too One sharing module resolves the share-with-* inputs to the UUIDs the ACL API requires and sends a single PATCH. Names resolve through the users and groups endpoints, UUIDs pass through, and the token owner is dropped because the API rejects a self-share. --- AGENTS.md | 5 +- README.md | 20 +++- action.yaml | 10 +- dist/index.js | 168 ++++++++++++++++++++++++++++--- scripts/typegen/main.go | 1 + src/action.ts | 47 +++------ src/coder-client.test.ts | 25 +++++ src/coder-client.ts | 41 +++++++- src/codersdk.gen.ts | 20 ++++ src/index.ts | 3 + src/schemas.test.ts | 19 +++- src/schemas.ts | 2 + src/sharing.test.ts | 204 ++++++++++++++++++++++++++++++++++++++ src/sharing.ts | 207 +++++++++++++++++++++++++++++++++++++++ src/test-helpers.ts | 33 +++++++ 15 files changed, 744 insertions(+), 61 deletions(-) create mode 100644 src/sharing.test.ts create mode 100644 src/sharing.ts diff --git a/AGENTS.md b/AGENTS.md index 2b2b41c..b957414 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,7 @@ CoderAgentChatAction.run() (action.ts) - **index.ts** - Entry point, parses GHA inputs, initializes clients, runs action - **action.ts** - Core business logic: user resolution, chat creation, issue commenting - **coder-client.ts** - Coder API client for Chat endpoints + user lookup +- **sharing.ts** - Resolves `share-with-*` inputs to the UUIDs the ACL API needs and grants read access on a new chat - **schemas.ts** - Zod schemas for action inputs and outputs ### Test Files (src/*.test.ts) @@ -103,4 +104,6 @@ bun run build - `GET /api/experimental/chats` - List chats - **Chat sharing**: - - `PATCH /api/v2/chats/{id}/acl` - Grant read access to users or groups (used by `share-with-organization`) + - `PATCH /api/v2/chats/{id}/acl` - Grant read access to users or groups (used by the `share-with-*` inputs) + - `GET /api/v2/users/{user}` - Resolve a username to its UUID + - `GET /api/v2/organizations/{organization}/groups/{groupName}` - Resolve a group name to its UUID (licensed deployments only) diff --git a/README.md b/README.md index 3d012bf..b4f1240 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,8 @@ The chat runs as whoever the `coder-token` belongs to; that identity is the only | `idempotency-key` | no | | Optional sharding key on the reuse scope. See [Chat reuse](#chat-reuse). | | `force-new-chat` | no | `false` | Skip chat-reuse lookup and always create. Mutually exclusive with `existing-chat-id`. | | `share-with-organization` | no | `false` | Give the chat's Coder organization read access to a newly created chat. See [Who can read the chat](#who-can-read-the-chat). | +| `share-with-groups` | no | | Coder groups, as names or UUIDs, comma or newline separated. Names need a licensed deployment. | +| `share-with-users` | no | | Coder users, as usernames or UUIDs, comma or newline separated. Each user is notified once per new chat. | ## Outputs @@ -117,16 +119,26 @@ Either path fails with `chat-error-kind=org_not_found` when the org doesn't exis Every chat is owned by the `coder-token` holder, usually a bot account. By default nobody else can open it, and the `chat-url` in the issue comment answers "Chat not found" for everyone but that bot. That hides the agent's reasoning from the people reading its output. -Set `share-with-organization: true` to grant the chat's organization read access. The Everyone group shares its organization's ID, so one group entry covers every member of the organization the chat runs in, resolved the same way as [Organization resolution](#organization-resolution) above. +Three inputs grant read access on a chat this run creates. They combine, and the action sends them as one request. + +```yaml + share-with-organization: true # everyone in the chat's organization + share-with-groups: docs, 0f1e... # group names or group UUIDs + share-with-users: nickvigilante # usernames or user UUIDs +``` + +`share-with-organization` needs no lookup: the Everyone group shares its organization's ID, and the organization is the one already resolved for `createChat` (see [Organization resolution](#organization-resolution)). Group names resolve through `GET /api/v2/organizations/{organization}/groups/{groupName}`, which only a licensed deployment serves; a group UUID skips the lookup and works everywhere. Usernames resolve through `GET /api/v2/users/{user}` on any deployment. Details worth knowing: - Read-only. Readers can open the chat and follow it; they cannot send messages. -- Only on creation. A reused chat keeps the access it already had, so turning the input on does not retroactively open past chats. +- Only on creation. A reused chat keeps the access it already had, so turning these inputs on does not retroactively open past chats. - Applied before the `wait: complete` poll, so a reader can watch a long run rather than only read it afterwards. -- Group access sends no notifications. Sharing a chat with a named user does. +- Groups are quiet, users are not. Coder notifies each user named in `user_roles` once per chat and never notifies group members. +- The `coder-token` owner is skipped in `share-with-users`. The API rejects a request that changes the caller's own role, and that rejection would drop every other entry in the same request. +- An entry that does not resolve is skipped with a warning; the rest still share. If nothing resolves, the chat stays private and the run logs a warning. - Sub-chats are separate. Coder sets ACLs on root chats only, so a subagent's chat is not covered by the parent's entry. -- A deployment with chat sharing disabled answers `403` here. The action logs a warning and the run still succeeds, because the chat itself is fine. +- A deployment with chat sharing disabled answers `403`. The action logs a warning and the run still succeeds, because the chat itself is fine. ### Chat reuse diff --git a/action.yaml b/action.yaml index 0c8ddb7..788709e 100644 --- a/action.yaml +++ b/action.yaml @@ -68,10 +68,18 @@ inputs: default: "false" share-with-organization: - description: "Give the chat's Coder organization read access to a newly created chat, so people other than the `coder-token` holder can open it. Only applies when this run creates the chat; a reused chat keeps whatever access it already had. Read-only, and group access sends no notifications. A deployment with chat sharing disabled logs a warning instead of failing the run." + description: "Give the chat's Coder organization read access to a newly created chat, so people other than the `coder-token` holder can open it. The Everyone group shares its organization's ID, so this needs no group lookup. Only applies when this run creates the chat; a reused chat keeps whatever access it already had. Read-only, and group access sends no notifications. A deployment with chat sharing disabled logs a warning instead of failing the run." required: false default: "false" + share-with-groups: + description: "Coder groups to give read access to a newly created chat, as names or UUIDs, separated by commas or newlines. Names resolve inside the chat's organization and need a licensed deployment; UUIDs work everywhere. An entry that does not resolve is skipped with a warning. Same creation-only and read-only rules as share-with-organization." + required: false + + share-with-users: + description: "Coder users to give read access to a newly created chat, as usernames or UUIDs, separated by commas or newlines. Each named user gets one notification per new chat; groups do not. The `coder-token` owner is skipped, since a chat cannot be shared with its own owner. Same creation-only and read-only rules as share-with-organization." + required: false + outputs: coder-username: description: "The Coder username the `coder-token` belongs to (always the chat owner; the chats API has no owner override)." diff --git a/dist/index.js b/dist/index.js index 4034c18..77e2e0b 100644 --- a/dist/index.js +++ b/dist/index.js @@ -36620,6 +36620,7 @@ var CreateChatRequestSchema = exports_external.object({ plan_mode: ChatPlanModeSchema.optional(), client_type: ChatClientTypeSchema.optional() }); +var GroupSourceSchema = exports_external.enum(["oidc", "user"]); var LoginTypeSchema = exports_external.enum([ "github", "none", @@ -36666,6 +36667,19 @@ var ReducedUserSchema = MinimalUserSchema.extend({ is_service_account: exports_external.boolean().optional(), theme_preference: exports_external.string().optional() }); +var GroupSchema = exports_external.object({ + id: exports_external.string(), + name: exports_external.string(), + display_name: exports_external.string(), + organization_id: exports_external.string(), + members: exports_external.array(ReducedUserSchema), + total_member_count: exports_external.number(), + avatar_url: exports_external.string(), + quota_allowance: exports_external.number(), + source: GroupSourceSchema, + organization_name: exports_external.string(), + organization_display_name: exports_external.string() +}); var UserSchema = ReducedUserSchema.extend({ organization_ids: exports_external.array(exports_external.string()), roles: exports_external.array(SlimRoleSchema), @@ -36729,6 +36743,22 @@ class RealCoderClient { const response = await this.request(endpoint2); return OrganizationSchema.parse(response); } + async getUser(usernameOrID) { + if (!usernameOrID) { + throw new CoderAPIError("User cannot be empty", 400); + } + const endpoint2 = `/api/v2/users/${encodeURIComponent(usernameOrID)}`; + const response = await this.request(endpoint2); + return UserSchema.parse(response); + } + async getGroupByName(organizationID, name) { + if (!organizationID || !name) { + throw new CoderAPIError("Organization and group name cannot be empty", 400); + } + const endpoint2 = `/api/v2/organizations/${encodeURIComponent(organizationID)}/groups/${encodeURIComponent(name)}?exclude_members=true`; + const response = await this.request(endpoint2); + return GroupSchema.parse(response); + } async createChat(params) { const endpoint2 = "/api/experimental/chats"; const response = await this.request(endpoint2, { @@ -37076,6 +37106,118 @@ function buildDeploymentAgentsUrl(coderURL) { return `${normalizeBaseUrl(coderURL)}/agents`; } +// src/sharing.ts +var UUIDSchema = exports_external.uuid(); +function isUUID(value) { + return UUIDSchema.safeParse(value).success; +} +function parseShareList(raw) { + if (!raw) { + return []; + } + const seen = new Set; + const out = []; + for (const part of raw.split(/[,\n]/)) { + const value = part.trim(); + if (value && !seen.has(value)) { + seen.add(value); + out.push(value); + } + } + return out; +} +function hasShareTargets(request2) { + return request2.organization || request2.groups.length > 0 || request2.users.length > 0; +} +async function resolveChatShare(coder, request2, ctx) { + const groupRoles = {}; + const userRoles = {}; + if (request2.organization) { + groupRoles[ctx.organizationID] = "read"; + } + for (const group of request2.groups) { + const id = await resolveGroupID(coder, ctx.organizationID, group); + if (id) { + groupRoles[id] = "read"; + } + } + for (const user of request2.users) { + const id = await resolveUserID(coder, user); + if (!id) { + continue; + } + if (id === ctx.tokenOwnerID) { + info(`Skipping share-with-users entry '${user}': it is the coder-token owner`); + continue; + } + userRoles[id] = "read"; + } + const acl = {}; + if (Object.keys(groupRoles).length > 0) { + acl.group_roles = groupRoles; + } + if (Object.keys(userRoles).length > 0) { + acl.user_roles = userRoles; + } + return acl.group_roles || acl.user_roles ? acl : null; +} +async function resolveGroupID(coder, organizationID, group) { + if (isUUID(group)) { + return group; + } + try { + const found = await coder.getGroupByName(organizationID, group); + return found.id; + } catch (error52) { + const hint = error52 instanceof CoderAPIError && error52.statusCode === 404 ? " Group lookup by name needs a licensed deployment; pass the group UUID instead." : ""; + warning(`Could not resolve share-with-groups entry '${group}': ${describe3(error52)}.${hint}`); + return; + } +} +async function resolveUserID(coder, user) { + if (isUUID(user)) { + return user; + } + try { + const found = await coder.getUser(user); + return found.id; + } catch (error52) { + warning(`Could not resolve share-with-users entry '${user}': ${describe3(error52)}`); + return; + } +} +async function shareNewChat(coder, chatId, request2, ctx) { + if (!hasShareTargets(request2)) { + return; + } + const acl = await resolveChatShare(coder, request2, ctx); + if (!acl) { + warning("No share-with-* entry resolved, so the chat was not shared"); + return; + } + try { + await coder.updateChatACL(chatId, acl); + info(`Granted read access on the chat to ${summarize(acl)}`); + } catch (error52) { + warning(`Could not share the chat: ${describe3(error52)}`); + } +} +function summarize(acl) { + const parts = []; + const groups = Object.keys(acl.group_roles ?? {}).length; + const users = Object.keys(acl.user_roles ?? {}).length; + if (groups) { + parts.push(`${groups} group${groups === 1 ? "" : "s"}`); + } + if (users) { + parts.push(`${users} user${users === 1 ? "" : "s"}`); + } + return parts.join(" and "); +} +function describe3(error52) { + return error52 instanceof Error ? error52.message : String(error52); +} + // src/action.ts var defaultClock = { now: () => Date.now(), @@ -37386,9 +37528,11 @@ class CoderAgentChatAction { info(`Agents chat created successfully (id: ${createdChat.id}, status: ${createdChat.status})`); const chatUrl = this.generateChatUrl(createdChat.id); info(`Chat URL: ${chatUrl}`); - if (this.inputs.shareWithOrganization) { - await this.shareWithOrganization(createdChat.id, organizationID); - } + await shareNewChat(this.coder, createdChat.id, { + organization: this.inputs.shareWithOrganization, + groups: this.inputs.shareWithGroups, + users: this.inputs.shareWithUsers + }, { organizationID, tokenOwnerID: tokenOwner.id }); let finalChat = createdChat; if (this.inputs.wait === "complete") { info(`Waiting for chat to reach terminal status (timeout: ${this.inputs.waitTimeoutSeconds}s)...`); @@ -37414,16 +37558,6 @@ class CoderAgentChatAction { } return this.buildOutputs(coderUsername, finalChat, true); } - async shareWithOrganization(chatId, organizationID) { - try { - await this.coder.updateChatACL(chatId, { - group_roles: { [organizationID]: "read" } - }); - info(`Granted read access on the chat to organization ${organizationID}`); - } catch (error52) { - warning(`Could not share the chat with organization ${organizationID}: ${error52 instanceof Error ? error52.message : String(error52)}`); - } - } async runFollowUp(args) { const { coderUsername, @@ -37592,7 +37726,9 @@ var ActionInputsObjectSchema = exports_external.object({ waitTimeoutSeconds: exports_external.coerce.number().int().positive().default(DEFAULT_WAIT_TIMEOUT_SECONDS), idempotencyKey: exports_external.string().min(1).optional(), forceNewChat: exports_external.boolean().default(false), - shareWithOrganization: exports_external.boolean().default(false) + shareWithOrganization: exports_external.boolean().default(false), + shareWithGroups: exports_external.array(exports_external.string().min(1)).default([]), + shareWithUsers: exports_external.array(exports_external.string().min(1)).default([]) }); var ActionInputsSchema = ActionInputsObjectSchema.refine((data) => !(data.existingChatId !== undefined && data.forceNewChat === true), { message: "Cannot set both existing-chat-id and force-new-chat; choose one.", @@ -37643,7 +37779,9 @@ async function main() { waitTimeoutSeconds: getInput("wait-timeout-seconds") || undefined, idempotencyKey: getInput("idempotency-key") || undefined, forceNewChat: getBooleanInput("force-new-chat"), - shareWithOrganization: getBooleanInput("share-with-organization") + shareWithOrganization: getBooleanInput("share-with-organization"), + shareWithGroups: parseShareList(getInput("share-with-groups")), + shareWithUsers: parseShareList(getInput("share-with-users")) }); debug("Inputs validated successfully"); debug(`Coder URL: ${inputs.coderURL}`); diff --git a/scripts/typegen/main.go b/scripts/typegen/main.go index edadd36..35fde41 100644 --- a/scripts/typegen/main.go +++ b/scripts/typegen/main.go @@ -28,6 +28,7 @@ var wantedTypes = map[string]bool{ "CreateChatMessageRequest": true, "CreateChatRequest": true, "UpdateChatACL": true, + "Group": true, "Organization": true, "User": true, } diff --git a/src/action.ts b/src/action.ts index c420f93..57eba34 100644 --- a/src/action.ts +++ b/src/action.ts @@ -24,6 +24,7 @@ import { upsertCommentByMarker, } from "./comment"; import type { ActionInputs, ActionOutputs, ChatErrorKind } from "./schemas"; +import { shareNewChat } from "./sharing"; export type Octokit = ReturnType; @@ -649,9 +650,16 @@ export class CoderAgentChatAction { // Share before polling, so the chat is readable while it runs // rather than only after it finishes. - if (this.inputs.shareWithOrganization) { - await this.shareWithOrganization(createdChat.id, organizationID); - } + await shareNewChat( + this.coder, + createdChat.id, + { + organization: this.inputs.shareWithOrganization, + groups: this.inputs.shareWithGroups, + users: this.inputs.shareWithUsers, + }, + { organizationID, tokenOwnerID: tokenOwner.id }, + ); // Poll before commenting so wait=complete posts only after the // chat reaches a terminal state. No mid-poll comment updates. @@ -687,39 +695,6 @@ export class CoderAgentChatAction { return this.buildOutputs(coderUsername, finalChat, true); } - /** - * Give the chat's organization read access, so anyone in it can open - * the chat this run created. The Everyone group shares its - * organization's ID, so a single group entry covers every member. - * - * Only the create branch calls this. A reused chat was shared by the - * run that created it, and a chat created before this input existed - * stays private on purpose: re-sharing it would be a silent change to - * who can read past work. - * - * A sharing failure never fails the run. The review itself is fine, - * and a deployment with chat sharing disabled answers 403 here. - */ - private async shareWithOrganization( - chatId: ChatId, - organizationID: string, - ): Promise { - try { - await this.coder.updateChatACL(chatId, { - group_roles: { [organizationID]: "read" }, - }); - core.info( - `Granted read access on the chat to organization ${organizationID}`, - ); - } catch (error) { - core.warning( - `Could not share the chat with organization ${organizationID}: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } - } - /** * Send `chat-prompt` as a follow-up message to an existing chat and * complete the post-message flow (poll under `wait: complete`, refresh diff --git a/src/coder-client.test.ts b/src/coder-client.test.ts index 2c60f6f..995ea53 100644 --- a/src/coder-client.test.ts +++ b/src/coder-client.test.ts @@ -9,6 +9,7 @@ import { mockChat, mockChatMessageResponse, mockOrganization, + mockGroup, createMockInputs, createMockResponse, } from "./test-helpers"; @@ -137,6 +138,30 @@ describe("CoderClient", () => { }); }); + describe("getUser", () => { + test("looks a user up by username or ID on the v2 route", async () => { + mockFetch.mockResolvedValue(createMockResponse(mockUser)); + const result = await client.getUser("nick"); + expect(result.id).toBe(mockUser.id); + expect(mockFetch).toHaveBeenCalledWith( + "https://coder.test/api/v2/users/nick", + expect.anything(), + ); + }); + }); + + describe("getGroupByName", () => { + test("looks a group up inside the organization without its members", async () => { + mockFetch.mockResolvedValue(createMockResponse(mockGroup)); + const result = await client.getGroupByName(mockOrganization.id, "docs"); + expect(result.id).toBe(mockGroup.id); + expect(mockFetch).toHaveBeenCalledWith( + `https://coder.test/api/v2/organizations/${mockOrganization.id}/groups/docs?exclude_members=true`, + expect.anything(), + ); + }); + }); + describe("updateChatACL", () => { test("patches the v2 ACL route with the group role", async () => { mockFetch.mockResolvedValue( diff --git a/src/coder-client.ts b/src/coder-client.ts index 72c9ab0..09c8d11 100644 --- a/src/coder-client.ts +++ b/src/coder-client.ts @@ -8,6 +8,7 @@ import { ChatStatusSchema, CreateChatMessageRequestSchema, CreateChatRequestSchema, + GroupSchema, OrganizationSchema, UpdateChatACLSchema, UserSchema, @@ -15,6 +16,7 @@ import { import type { CreateChatMessageRequest, CreateChatRequest, + Group, Organization, UpdateChatACL, User, @@ -38,6 +40,7 @@ export { ChatStatusSchema, CreateChatMessageRequestSchema, CreateChatRequestSchema, + GroupSchema, OrganizationSchema, UpdateChatACLSchema, UserSchema, @@ -50,6 +53,7 @@ export type { ChatStatus, CreateChatMessageRequest, CreateChatRequest, + Group, Organization, UpdateChatACL, User, @@ -84,6 +88,18 @@ export interface CoderClient { getOrganizationByName(name: string): Promise; + /** + * Resolve a user by username or UUID via `GET /api/v2/users/{user}`. + */ + getUser(usernameOrID: string): Promise; + + /** + * Resolve a group by name inside an organization via + * `GET /api/v2/organizations/{organization}/groups/{groupName}`. Served + * by the licensed build only; unlicensed deployments answer 404. + */ + getGroupByName(organizationID: string, name: string): Promise; + createChat(params: CreateChatRequest): Promise; createChatMessage( @@ -99,7 +115,8 @@ export interface CoderClient { * Grant read access on a chat to users or groups via * `PATCH /api/v2/chats/{chat}/acl`. Every chat is owned by the * `coder-token` holder, so without an ACL entry nobody else can open - * one. Group entries send no notifications; user entries do. + * one. Keys must be existing UUIDs. Group entries send no + * notifications; user entries do. */ updateChatACL(chatId: ChatId, params: UpdateChatACL): Promise; } @@ -190,6 +207,28 @@ export class RealCoderClient implements CoderClient { return OrganizationSchema.parse(response); } + async getUser(usernameOrID: string): Promise { + if (!usernameOrID) { + throw new CoderAPIError("User cannot be empty", 400); + } + const endpoint = `/api/v2/users/${encodeURIComponent(usernameOrID)}`; + const response = await this.request(endpoint); + return UserSchema.parse(response); + } + + async getGroupByName(organizationID: string, name: string): Promise { + if (!organizationID || !name) { + throw new CoderAPIError( + "Organization and group name cannot be empty", + 400, + ); + } + // Only the ID is needed, so leave the member list out of the response. + const endpoint = `/api/v2/organizations/${encodeURIComponent(organizationID)}/groups/${encodeURIComponent(name)}?exclude_members=true`; + const response = await this.request(endpoint); + return GroupSchema.parse(response); + } + async createChat(params: CreateChatRequest): Promise { const endpoint = "/api/experimental/chats"; const response = await this.request(endpoint, { diff --git a/src/codersdk.gen.ts b/src/codersdk.gen.ts index e1ea95b..5623ed0 100644 --- a/src/codersdk.gen.ts +++ b/src/codersdk.gen.ts @@ -178,6 +178,10 @@ export const CreateChatRequestSchema = z.object({ export type CreateChatRequest = z.infer; +export const GroupSourceSchema = z.enum(["oidc", "user"]); + +export type GroupSource = z.infer; + export const LoginTypeSchema = z.enum([ "github", "none", @@ -248,6 +252,22 @@ export const ReducedUserSchema = MinimalUserSchema.extend({ export type ReducedUser = z.infer; +export const GroupSchema = z.object({ + id: z.string(), + name: z.string(), + display_name: z.string(), + organization_id: z.string(), + members: z.array(ReducedUserSchema), + total_member_count: z.number(), + avatar_url: z.string(), + quota_allowance: z.number(), + source: GroupSourceSchema, + organization_name: z.string(), + organization_display_name: z.string(), +}); + +export type Group = z.infer; + export const UserSchema = ReducedUserSchema.extend({ organization_ids: z.array(z.string()), roles: z.array(SlimRoleSchema), diff --git a/src/index.ts b/src/index.ts index db085fc..480046e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,6 +4,7 @@ import { ActionFailureError, CoderAgentChatAction } from "./action"; import { RealCoderClient } from "./coder-client"; import { setActionOutputs, setFailureOutputs } from "./outputs"; import { ActionInputsSchema } from "./schemas"; +import { parseShareList } from "./sharing"; async function main() { try { @@ -23,6 +24,8 @@ async function main() { idempotencyKey: core.getInput("idempotency-key") || undefined, forceNewChat: core.getBooleanInput("force-new-chat"), shareWithOrganization: core.getBooleanInput("share-with-organization"), + shareWithGroups: parseShareList(core.getInput("share-with-groups")), + shareWithUsers: parseShareList(core.getInput("share-with-users")), }); core.debug("Inputs validated successfully"); diff --git a/src/schemas.test.ts b/src/schemas.test.ts index 335dc20..65e4ea0 100644 --- a/src/schemas.test.ts +++ b/src/schemas.test.ts @@ -20,6 +20,8 @@ const actionInputValid: ActionInputs = { waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, forceNewChat: false, shareWithOrganization: false, + shareWithGroups: [], + shareWithUsers: [], }; describe("ActionInputsSchema", () => { @@ -97,11 +99,22 @@ describe("ActionInputsSchema", () => { } }); - test("shareWithOrganization defaults to false when omitted", () => { - const { shareWithOrganization, ...withoutShare } = actionInputValid; - expect(shareWithOrganization).toBe(false); + test("share-with-* inputs default to off and empty when omitted", () => { + const { + shareWithOrganization, + shareWithGroups, + shareWithUsers, + ...withoutShare + } = actionInputValid; + expect([shareWithOrganization, shareWithGroups, shareWithUsers]).toEqual([ + false, + [], + [], + ]); const result = ActionInputsSchema.parse(withoutShare); expect(result.shareWithOrganization).toBe(false); + expect(result.shareWithGroups).toEqual([]); + expect(result.shareWithUsers).toEqual([]); }); }); diff --git a/src/schemas.ts b/src/schemas.ts index 75d890b..eb326b0 100644 --- a/src/schemas.ts +++ b/src/schemas.ts @@ -24,6 +24,8 @@ const ActionInputsObjectSchema = z.object({ idempotencyKey: z.string().min(1).optional(), forceNewChat: z.boolean().default(false), shareWithOrganization: z.boolean().default(false), + shareWithGroups: z.array(z.string().min(1)).default([]), + shareWithUsers: z.array(z.string().min(1)).default([]), }); export const ActionInputsSchema = ActionInputsObjectSchema.refine( diff --git a/src/sharing.test.ts b/src/sharing.test.ts new file mode 100644 index 0000000..ff69b18 --- /dev/null +++ b/src/sharing.test.ts @@ -0,0 +1,204 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import * as core from "@actions/core"; +import { CoderAPIError } from "./coder-client"; +import { + hasShareTargets, + parseShareList, + resolveChatShare, + shareNewChat, +} from "./sharing"; +import { + MockCoderClient, + mockChat, + mockGroup, + mockOrganization, + mockUser, +} from "./test-helpers"; + +const ORG = mockOrganization.id; +const OWNER = mockUser.id; +const OTHER_USER_ID = "ee0e8400-e29b-41d4-a716-446655440000"; +const ctx = { organizationID: ORG, tokenOwnerID: OWNER }; + +describe("parseShareList", () => { + test("returns an empty list for an unset input", () => { + expect(parseShareList(undefined)).toEqual([]); + expect(parseShareList("")).toEqual([]); + }); + + test("splits on commas and newlines, trims, and drops duplicates", () => { + expect(parseShareList(" docs, platform\n\ndocs ,\n platform-2 ")).toEqual([ + "docs", + "platform", + "platform-2", + ]); + }); +}); + +describe("hasShareTargets", () => { + test("is false only when every input is empty", () => { + expect( + hasShareTargets({ organization: false, groups: [], users: [] }), + ).toBe(false); + expect(hasShareTargets({ organization: true, groups: [], users: [] })).toBe( + true, + ); + expect( + hasShareTargets({ organization: false, groups: ["docs"], users: [] }), + ).toBe(true); + expect( + hasShareTargets({ organization: false, groups: [], users: ["ben"] }), + ).toBe(true); + }); +}); + +describe("resolveChatShare", () => { + let coder: MockCoderClient; + let warning: ReturnType; + + beforeEach(() => { + coder = new MockCoderClient(); + warning = spyOn(core, "warning").mockImplementation(() => {}); + }); + + afterEach(() => { + warning.mockRestore(); + }); + + test("organization becomes one group entry keyed by the org ID, with no lookup", async () => { + const acl = await resolveChatShare( + coder, + { organization: true, groups: [], users: [] }, + ctx, + ); + expect(acl).toEqual({ group_roles: { [ORG]: "read" } }); + expect(coder.mockGetGroupByName).not.toHaveBeenCalled(); + }); + + test("group UUIDs pass through and names resolve inside the chat's org", async () => { + const acl = await resolveChatShare( + coder, + { + organization: false, + groups: ["ff0e8400-e29b-41d4-a716-446655440000", "docs"], + users: [], + }, + ctx, + ); + expect(coder.mockGetGroupByName).toHaveBeenCalledTimes(1); + expect(coder.mockGetGroupByName).toHaveBeenCalledWith(ORG, "docs"); + expect(acl).toEqual({ + group_roles: { + "ff0e8400-e29b-41d4-a716-446655440000": "read", + [mockGroup.id]: "read", + }, + }); + }); + + test("user UUIDs pass through and usernames resolve", async () => { + coder.mockGetUser.mockResolvedValue({ ...mockUser, id: OTHER_USER_ID }); + const acl = await resolveChatShare( + coder, + { organization: false, groups: [], users: ["nick"] }, + ctx, + ); + expect(coder.mockGetUser).toHaveBeenCalledWith("nick"); + expect(acl).toEqual({ user_roles: { [OTHER_USER_ID]: "read" } }); + }); + + test("drops the token owner from users, since the API rejects a self-share", async () => { + coder.mockGetUser.mockResolvedValue(mockUser); + const acl = await resolveChatShare( + coder, + { organization: true, groups: [], users: [OWNER, mockUser.username] }, + ctx, + ); + // The org entry survives; the owner never reaches user_roles. + expect(acl).toEqual({ group_roles: { [ORG]: "read" } }); + }); + + test("an unresolvable name is warned about and skipped, the rest still share", async () => { + coder.mockGetGroupByName.mockRejectedValue( + new CoderAPIError("Coder API error: Not Found", 404), + ); + const acl = await resolveChatShare( + coder, + { organization: true, groups: ["nope"], users: [] }, + ctx, + ); + expect(acl).toEqual({ group_roles: { [ORG]: "read" } }); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("needs a licensed deployment"), + ); + }); + + test("returns null when nothing resolves", async () => { + coder.mockGetUser.mockRejectedValue( + new CoderAPIError("Coder API error: Not Found", 404), + ); + const acl = await resolveChatShare( + coder, + { organization: false, groups: [], users: ["ghost"] }, + ctx, + ); + expect(acl).toBeNull(); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("share-with-users entry 'ghost'"), + ); + }); +}); + +describe("shareNewChat", () => { + let coder: MockCoderClient; + let warning: ReturnType; + + beforeEach(() => { + coder = new MockCoderClient(); + warning = spyOn(core, "warning").mockImplementation(() => {}); + }); + + afterEach(() => { + warning.mockRestore(); + }); + + test("does nothing when no share input is set", async () => { + await shareNewChat( + coder, + mockChat.id, + { organization: false, groups: [], users: [] }, + ctx, + ); + expect(coder.mockUpdateChatACL).not.toHaveBeenCalled(); + expect(warning).not.toHaveBeenCalled(); + }); + + test("sends one PATCH carrying every resolved entry", async () => { + coder.mockGetUser.mockResolvedValue({ ...mockUser, id: OTHER_USER_ID }); + await shareNewChat( + coder, + mockChat.id, + { organization: true, groups: ["docs"], users: ["nick"] }, + ctx, + ); + expect(coder.mockUpdateChatACL).toHaveBeenCalledTimes(1); + expect(coder.mockUpdateChatACL).toHaveBeenCalledWith(mockChat.id, { + group_roles: { [ORG]: "read", [mockGroup.id]: "read" }, + user_roles: { [OTHER_USER_ID]: "read" }, + }); + }); + + test("warns instead of throwing when the PATCH is rejected", async () => { + coder.mockUpdateChatACL.mockRejectedValue( + new CoderAPIError("Chat sharing is disabled for this deployment.", 403), + ); + await shareNewChat( + coder, + mockChat.id, + { organization: true, groups: [], users: [] }, + ctx, + ); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("Could not share the chat"), + ); + }); +}); diff --git a/src/sharing.ts b/src/sharing.ts new file mode 100644 index 0000000..a9066af --- /dev/null +++ b/src/sharing.ts @@ -0,0 +1,207 @@ +import * as core from "@actions/core"; +import { z } from "zod"; +import { + type ChatId, + CoderAPIError, + type CoderClient, + type UpdateChatACL, +} from "./coder-client"; + +/** + * Who a newly created chat should be readable by, straight from the + * `share-with-*` inputs. Groups and users may be names or UUIDs. + */ +export interface ShareRequest { + organization: boolean; + groups: string[]; + users: string[]; +} + +/** + * Facts the resolver needs that only the create path knows. + */ +export interface ShareContext { + /** Organization the chat was created in. Doubles as the Everyone group ID. */ + organizationID: string; + /** The `coder-token` holder. The API rejects a request that names its own caller. */ + tokenOwnerID: string; +} + +const UUIDSchema = z.uuid(); + +function isUUID(value: string): boolean { + return UUIDSchema.safeParse(value).success; +} + +/** + * Split a `share-with-groups` or `share-with-users` input. Accepts commas, + * newlines, or both, so a one-line YAML value and a block scalar both work. + */ +export function parseShareList(raw: string | undefined): string[] { + if (!raw) { + return []; + } + const seen = new Set(); + const out: string[] = []; + for (const part of raw.split(/[,\n]/)) { + const value = part.trim(); + if (value && !seen.has(value)) { + seen.add(value); + out.push(value); + } + } + return out; +} + +export function hasShareTargets(request: ShareRequest): boolean { + return ( + request.organization || + request.groups.length > 0 || + request.users.length > 0 + ); +} + +/** + * Turn names into the UUIDs the ACL API requires. Every entry the API + * receives must be an existing UUID, so anything that fails to resolve is + * dropped with a warning rather than sent along to fail the whole PATCH. + * Returns null when nothing is left to share. + */ +export async function resolveChatShare( + coder: CoderClient, + request: ShareRequest, + ctx: ShareContext, +): Promise { + const groupRoles: Record = {}; + const userRoles: Record = {}; + + if (request.organization) { + // The Everyone group shares its organization's ID, so this needs no + // lookup and works on deployments without the groups API. + groupRoles[ctx.organizationID] = "read"; + } + + for (const group of request.groups) { + const id = await resolveGroupID(coder, ctx.organizationID, group); + if (id) { + groupRoles[id] = "read"; + } + } + + for (const user of request.users) { + const id = await resolveUserID(coder, user); + if (!id) { + continue; + } + if (id === ctx.tokenOwnerID) { + // The owner already reads their own chat, and the API answers 400 + // to a request that changes the caller's own role, which would + // take every other entry in this PATCH down with it. + core.info( + `Skipping share-with-users entry '${user}': it is the coder-token owner`, + ); + continue; + } + userRoles[id] = "read"; + } + + const acl: UpdateChatACL = {}; + if (Object.keys(groupRoles).length > 0) { + acl.group_roles = groupRoles; + } + if (Object.keys(userRoles).length > 0) { + acl.user_roles = userRoles; + } + return acl.group_roles || acl.user_roles ? acl : null; +} + +async function resolveGroupID( + coder: CoderClient, + organizationID: string, + group: string, +): Promise { + if (isUUID(group)) { + return group; + } + try { + const found = await coder.getGroupByName(organizationID, group); + return found.id; + } catch (error) { + // Group lookup by name is served by the licensed build only. A UUID + // skips the lookup, so name the workaround in the warning. + const hint = + error instanceof CoderAPIError && error.statusCode === 404 + ? " Group lookup by name needs a licensed deployment; pass the group UUID instead." + : ""; + core.warning( + `Could not resolve share-with-groups entry '${group}': ${describe(error)}.${hint}`, + ); + return undefined; + } +} + +async function resolveUserID( + coder: CoderClient, + user: string, +): Promise { + if (isUUID(user)) { + return user; + } + try { + const found = await coder.getUser(user); + return found.id; + } catch (error) { + core.warning( + `Could not resolve share-with-users entry '${user}': ${describe(error)}`, + ); + return undefined; + } +} + +/** + * Grant read access on a chat this run just created. Resolution and the + * PATCH both warn instead of throwing: the chat itself is fine, and a + * deployment with chat sharing disabled answers 403 here. + * + * Only the create path calls this. A reused chat keeps the access it + * already had, so turning these inputs on does not retroactively open + * chats created before them. + */ +export async function shareNewChat( + coder: CoderClient, + chatId: ChatId, + request: ShareRequest, + ctx: ShareContext, +): Promise { + if (!hasShareTargets(request)) { + return; + } + const acl = await resolveChatShare(coder, request, ctx); + if (!acl) { + core.warning("No share-with-* entry resolved, so the chat was not shared"); + return; + } + try { + await coder.updateChatACL(chatId, acl); + core.info(`Granted read access on the chat to ${summarize(acl)}`); + } catch (error) { + core.warning(`Could not share the chat: ${describe(error)}`); + } +} + +function summarize(acl: UpdateChatACL): string { + const parts: string[] = []; + const groups = Object.keys(acl.group_roles ?? {}).length; + const users = Object.keys(acl.user_roles ?? {}).length; + if (groups) { + parts.push(`${groups} group${groups === 1 ? "" : "s"}`); + } + if (users) { + parts.push(`${users} user${users === 1 ? "" : "s"}`); + } + return parts.join(" and "); +} + +function describe(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/test-helpers.ts b/src/test-helpers.ts index f2820ac..a091364 100644 --- a/src/test-helpers.ts +++ b/src/test-helpers.ts @@ -7,6 +7,7 @@ import { import type { User, CoderChat, + Group, Organization, CreateChatRequest, CreateChatMessageRequest, @@ -73,6 +74,22 @@ export const mockOrganization: Organization = { is_default: true, }; +// A named group in `mockOrganization`. Members are left out, matching +// the `exclude_members=true` response the client asks for. +export const mockGroup: Group = { + id: "dd0e8400-e29b-41d4-a716-446655440000", + name: "docs", + display_name: "Docs", + organization_id: mockOrganization.id, + members: [], + total_member_count: 3, + avatar_url: "", + quota_allowance: 0, + source: "user", + organization_name: mockOrganization.name, + organization_display_name: mockOrganization.display_name, +}; + export const mockChat: CoderChat = { id: ChatIdSchema.parse("990e8400-e29b-41d4-a716-446655440000"), organization_id: "660e8400-e29b-41d4-a716-446655440000", @@ -145,6 +162,8 @@ export function createMockInputs( waitTimeoutSeconds: DEFAULT_WAIT_TIMEOUT_SECONDS, forceNewChat: false, shareWithOrganization: false, + shareWithGroups: [], + shareWithUsers: [], ...overrides, } as ActionInputs; } @@ -163,6 +182,12 @@ export class MockCoderClient implements CoderClient { Promise.resolve([] as CoderChat[]), ); public mockGetAuthenticatedUser = mock(() => Promise.resolve(mockUser)); + public mockGetUser = mock((_usernameOrID: string) => + Promise.resolve(mockUser), + ); + public mockGetGroupByName = mock((_organizationID: string, _name: string) => + Promise.resolve(mockGroup), + ); public mockUpdateChatACL = mock((_chatId: ChatId, _params: UpdateChatACL) => Promise.resolve(), ); @@ -175,6 +200,14 @@ export class MockCoderClient implements CoderClient { return this.mockGetOrganizationByName(name); } + async getUser(usernameOrID: string): Promise { + return this.mockGetUser(usernameOrID); + } + + async getGroupByName(organizationID: string, name: string): Promise { + return this.mockGetGroupByName(organizationID, name); + } + async createChat(params: CreateChatRequest): Promise { return this.mockCreateChat(params); } From a2dae575f0d7a474192b3e79b2bcdba44f358089 Mon Sep 17 00:00:00 2001 From: Ben Potter Date: Fri, 11 Sep 2026 17:29:07 +0000 Subject: [PATCH 3/3] refactor: one resolver for groups and users, resolved in parallel --- dist/index.js | 112 ++++++++++------------------ src/sharing.test.ts | 2 +- src/sharing.ts | 175 +++++++++++++++++++------------------------- 3 files changed, 113 insertions(+), 176 deletions(-) diff --git a/dist/index.js b/dist/index.js index 77e2e0b..d0e750b 100644 --- a/dist/index.js +++ b/dist/index.js @@ -37108,83 +37108,40 @@ function buildDeploymentAgentsUrl(coderURL) { // src/sharing.ts var UUIDSchema = exports_external.uuid(); -function isUUID(value) { - return UUIDSchema.safeParse(value).success; -} function parseShareList(raw) { if (!raw) { return []; } - const seen = new Set; - const out = []; - for (const part of raw.split(/[,\n]/)) { - const value = part.trim(); - if (value && !seen.has(value)) { - seen.add(value); - out.push(value); - } - } - return out; + const values = raw.split(/[,\n]/).map((part) => part.trim()).filter(Boolean); + return [...new Set(values)]; } function hasShareTargets(request2) { return request2.organization || request2.groups.length > 0 || request2.users.length > 0; } async function resolveChatShare(coder, request2, ctx) { - const groupRoles = {}; - const userRoles = {}; + const [groupIDs, userIDs] = await Promise.all([ + resolveAll(request2.groups, (group) => resolveID("share-with-groups", group, async () => { + const found = await coder.getGroupByName(ctx.organizationID, group); + return found.id; + })), + resolveAll(request2.users, (user) => resolveID("share-with-users", user, async () => { + const found = await coder.getUser(user); + return found.id; + })) + ]); if (request2.organization) { - groupRoles[ctx.organizationID] = "read"; + groupIDs.add(ctx.organizationID); } - for (const group of request2.groups) { - const id = await resolveGroupID(coder, ctx.organizationID, group); - if (id) { - groupRoles[id] = "read"; - } + if (userIDs.delete(ctx.tokenOwnerID)) { + info("Skipping the coder-token owner in share-with-users"); } - for (const user of request2.users) { - const id = await resolveUserID(coder, user); - if (!id) { - continue; - } - if (id === ctx.tokenOwnerID) { - info(`Skipping share-with-users entry '${user}': it is the coder-token owner`); - continue; - } - userRoles[id] = "read"; - } - const acl = {}; - if (Object.keys(groupRoles).length > 0) { - acl.group_roles = groupRoles; - } - if (Object.keys(userRoles).length > 0) { - acl.user_roles = userRoles; - } - return acl.group_roles || acl.user_roles ? acl : null; -} -async function resolveGroupID(coder, organizationID, group) { - if (isUUID(group)) { - return group; - } - try { - const found = await coder.getGroupByName(organizationID, group); - return found.id; - } catch (error52) { - const hint = error52 instanceof CoderAPIError && error52.statusCode === 404 ? " Group lookup by name needs a licensed deployment; pass the group UUID instead." : ""; - warning(`Could not resolve share-with-groups entry '${group}': ${describe3(error52)}.${hint}`); - return; - } -} -async function resolveUserID(coder, user) { - if (isUUID(user)) { - return user; - } - try { - const found = await coder.getUser(user); - return found.id; - } catch (error52) { - warning(`Could not resolve share-with-users entry '${user}': ${describe3(error52)}`); - return; + if (groupIDs.size === 0 && userIDs.size === 0) { + return null; } + return { + ...groupIDs.size > 0 && { group_roles: readRoles(groupIDs) }, + ...userIDs.size > 0 && { user_roles: readRoles(userIDs) } + }; } async function shareNewChat(coder, chatId, request2, ctx) { if (!hasShareTargets(request2)) { @@ -37197,22 +37154,29 @@ async function shareNewChat(coder, chatId, request2, ctx) { } try { await coder.updateChatACL(chatId, acl); - info(`Granted read access on the chat to ${summarize(acl)}`); + info(`Granted read access on the chat to ${Object.keys(acl.group_roles ?? {}).length} group(s) and ${Object.keys(acl.user_roles ?? {}).length} user(s)`); } catch (error52) { warning(`Could not share the chat: ${describe3(error52)}`); } } -function summarize(acl) { - const parts = []; - const groups = Object.keys(acl.group_roles ?? {}).length; - const users = Object.keys(acl.user_roles ?? {}).length; - if (groups) { - parts.push(`${groups} group${groups === 1 ? "" : "s"}`); +async function resolveAll(values, resolve) { + const ids = await Promise.all(values.map(resolve)); + return new Set(ids.filter((id) => id !== undefined)); +} +async function resolveID(input, value, lookup) { + if (UUIDSchema.safeParse(value).success) { + return value; } - if (users) { - parts.push(`${users} user${users === 1 ? "" : "s"}`); + try { + return await lookup(); + } catch (error52) { + const hint = input === "share-with-groups" && error52 instanceof CoderAPIError && error52.statusCode === 404 ? " Either the group does not exist, or this deployment is unlicensed and cannot look groups up by name; a group UUID works in both cases." : ""; + warning(`Could not resolve ${input} entry '${value}': ${describe3(error52)}.${hint}`); + return; } - return parts.join(" and "); +} +function readRoles(ids) { + return Object.fromEntries([...ids].map((id) => [id, "read"])); } function describe3(error52) { return error52 instanceof Error ? error52.message : String(error52); diff --git a/src/sharing.test.ts b/src/sharing.test.ts index ff69b18..e3c530a 100644 --- a/src/sharing.test.ts +++ b/src/sharing.test.ts @@ -128,7 +128,7 @@ describe("resolveChatShare", () => { ); expect(acl).toEqual({ group_roles: { [ORG]: "read" } }); expect(warning).toHaveBeenCalledWith( - expect.stringContaining("needs a licensed deployment"), + expect.stringContaining("group UUID works in both cases"), ); }); diff --git a/src/sharing.ts b/src/sharing.ts index a9066af..281ee2a 100644 --- a/src/sharing.ts +++ b/src/sharing.ts @@ -29,10 +29,6 @@ export interface ShareContext { const UUIDSchema = z.uuid(); -function isUUID(value: string): boolean { - return UUIDSchema.safeParse(value).success; -} - /** * Split a `share-with-groups` or `share-with-users` input. Accepts commas, * newlines, or both, so a one-line YAML value and a block scalar both work. @@ -41,16 +37,11 @@ export function parseShareList(raw: string | undefined): string[] { if (!raw) { return []; } - const seen = new Set(); - const out: string[] = []; - for (const part of raw.split(/[,\n]/)) { - const value = part.trim(); - if (value && !seen.has(value)) { - seen.add(value); - out.push(value); - } - } - return out; + const values = raw + .split(/[,\n]/) + .map((part) => part.trim()) + .filter(Boolean); + return [...new Set(values)]; } export function hasShareTargets(request: ShareRequest): boolean { @@ -62,8 +53,8 @@ export function hasShareTargets(request: ShareRequest): boolean { } /** - * Turn names into the UUIDs the ACL API requires. Every entry the API - * receives must be an existing UUID, so anything that fails to resolve is + * Turn names into the UUIDs the ACL API requires. Every key the API + * receives must be an existing UUID, so an entry that fails to resolve is * dropped with a warning rather than sent along to fail the whole PATCH. * Returns null when nothing is left to share. */ @@ -72,90 +63,41 @@ export async function resolveChatShare( request: ShareRequest, ctx: ShareContext, ): Promise { - const groupRoles: Record = {}; - const userRoles: Record = {}; + const [groupIDs, userIDs] = await Promise.all([ + resolveAll(request.groups, (group) => + resolveID("share-with-groups", group, async () => { + const found = await coder.getGroupByName(ctx.organizationID, group); + return found.id; + }), + ), + resolveAll(request.users, (user) => + resolveID("share-with-users", user, async () => { + const found = await coder.getUser(user); + return found.id; + }), + ), + ]); if (request.organization) { // The Everyone group shares its organization's ID, so this needs no // lookup and works on deployments without the groups API. - groupRoles[ctx.organizationID] = "read"; + groupIDs.add(ctx.organizationID); } - for (const group of request.groups) { - const id = await resolveGroupID(coder, ctx.organizationID, group); - if (id) { - groupRoles[id] = "read"; - } + // The owner already reads their own chat, and the API answers 400 to a + // request that changes the caller's own role, which would take every + // other entry in this PATCH down with it. + if (userIDs.delete(ctx.tokenOwnerID)) { + core.info("Skipping the coder-token owner in share-with-users"); } - for (const user of request.users) { - const id = await resolveUserID(coder, user); - if (!id) { - continue; - } - if (id === ctx.tokenOwnerID) { - // The owner already reads their own chat, and the API answers 400 - // to a request that changes the caller's own role, which would - // take every other entry in this PATCH down with it. - core.info( - `Skipping share-with-users entry '${user}': it is the coder-token owner`, - ); - continue; - } - userRoles[id] = "read"; - } - - const acl: UpdateChatACL = {}; - if (Object.keys(groupRoles).length > 0) { - acl.group_roles = groupRoles; - } - if (Object.keys(userRoles).length > 0) { - acl.user_roles = userRoles; - } - return acl.group_roles || acl.user_roles ? acl : null; -} - -async function resolveGroupID( - coder: CoderClient, - organizationID: string, - group: string, -): Promise { - if (isUUID(group)) { - return group; - } - try { - const found = await coder.getGroupByName(organizationID, group); - return found.id; - } catch (error) { - // Group lookup by name is served by the licensed build only. A UUID - // skips the lookup, so name the workaround in the warning. - const hint = - error instanceof CoderAPIError && error.statusCode === 404 - ? " Group lookup by name needs a licensed deployment; pass the group UUID instead." - : ""; - core.warning( - `Could not resolve share-with-groups entry '${group}': ${describe(error)}.${hint}`, - ); - return undefined; - } -} - -async function resolveUserID( - coder: CoderClient, - user: string, -): Promise { - if (isUUID(user)) { - return user; - } - try { - const found = await coder.getUser(user); - return found.id; - } catch (error) { - core.warning( - `Could not resolve share-with-users entry '${user}': ${describe(error)}`, - ); - return undefined; + if (groupIDs.size === 0 && userIDs.size === 0) { + return null; } + return { + ...(groupIDs.size > 0 && { group_roles: readRoles(groupIDs) }), + ...(userIDs.size > 0 && { user_roles: readRoles(userIDs) }), + }; } /** @@ -183,23 +125,54 @@ export async function shareNewChat( } try { await coder.updateChatACL(chatId, acl); - core.info(`Granted read access on the chat to ${summarize(acl)}`); + core.info( + `Granted read access on the chat to ${Object.keys(acl.group_roles ?? {}).length} group(s) and ${Object.keys(acl.user_roles ?? {}).length} user(s)`, + ); } catch (error) { core.warning(`Could not share the chat: ${describe(error)}`); } } -function summarize(acl: UpdateChatACL): string { - const parts: string[] = []; - const groups = Object.keys(acl.group_roles ?? {}).length; - const users = Object.keys(acl.user_roles ?? {}).length; - if (groups) { - parts.push(`${groups} group${groups === 1 ? "" : "s"}`); +async function resolveAll( + values: string[], + resolve: (value: string) => Promise, +): Promise> { + const ids = await Promise.all(values.map(resolve)); + return new Set(ids.filter((id): id is string => id !== undefined)); +} + +/** + * A UUID is used as given. Anything else goes through `lookup`, and a + * failed lookup becomes a warning naming the input and the entry. + */ +async function resolveID( + input: string, + value: string, + lookup: () => Promise, +): Promise { + if (UUIDSchema.safeParse(value).success) { + return value; } - if (users) { - parts.push(`${users} user${users === 1 ? "" : "s"}`); + try { + return await lookup(); + } catch (error) { + // Group lookup by name is only served by the licensed build, so a 404 + // there is ambiguous. A UUID skips the lookup either way. + const hint = + input === "share-with-groups" && + error instanceof CoderAPIError && + error.statusCode === 404 + ? " Either the group does not exist, or this deployment is unlicensed and cannot look groups up by name; a group UUID works in both cases." + : ""; + core.warning( + `Could not resolve ${input} entry '${value}': ${describe(error)}.${hint}`, + ); + return undefined; } - return parts.join(" and "); +} + +function readRoles(ids: Set): Record { + return Object.fromEntries([...ids].map((id) => [id, "read" as const])); } function describe(error: unknown): string {