From fc8b725c4fcc047768e8c37dd103899613cfba2f Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 20 Mar 2026 09:55:45 +0000 Subject: [PATCH 1/3] test: teach the test suite to stop living dangerously - Add await to all .rejects.toThrow() and .resolves.toBeUndefined() assertions (unawaited promises are basically optimistic nihilism) - Add mockOctokitCommentingHappyPath helper to DRY up Octokit mock setup and eliminate spurious ::error:: output from integration tests - Wire up Octokit comment assertions in all integration tests so we actually verify the GitHub comment side-effects happen - Add afterEach in coder-client tests to restore global.fetch like a responsible adult - Add coverage for listChats, getCoderUserByGitHubId(undefined), existing chat message failure, commentOnIssue=false on existing chat path, and ActionOutputsSchema validation - No production code was harmed in the making of this commit --- src/action.test.ts | 64 ++++++++++++++++++++++++++++++++++------ src/coder-client.test.ts | 41 +++++++++++++++++++++---- src/schemas.test.ts | 50 ++++++++++++++++++++++++++++++- src/test-helpers.ts | 11 +++++++ 4 files changed, 150 insertions(+), 16 deletions(-) diff --git a/src/action.test.ts b/src/action.test.ts index 16607e4..43e6d54 100644 --- a/src/action.test.ts +++ b/src/action.test.ts @@ -5,6 +5,7 @@ import { ActionOutputsSchema } from "./schemas"; import { MockCoderClient, createMockOctokit, + mockOctokitCommentingHappyPath, createMockInputs, mockUser, mockChat, @@ -182,7 +183,7 @@ describe("CoderAgentChatAction", () => { inputs, ); - expect( + await expect( action.commentOnIssue("url", "owner", "repo", 123), ).resolves.toBeUndefined(); }); @@ -191,6 +192,7 @@ describe("CoderAgentChatAction", () => { test("creates new chat successfully", async () => { coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); + mockOctokitCommentingHappyPath(octokit); const inputs = createMockInputs({ githubUserID: 12345, @@ -216,10 +218,12 @@ describe("CoderAgentChatAction", () => { expect(parsedResult.chatUrl).toMatch( /^https:\/\/coder\.test\/chats\/[a-f0-9-]+$/, ); + expect(octokit.rest.issues.createComment).toHaveBeenCalled(); }); test("creates chat using direct coder-username", async () => { coderClient.mockCreateChat.mockResolvedValue(mockChat); + mockOctokitCommentingHappyPath(octokit); const inputs = createMockInputs({ githubUserID: undefined, @@ -239,6 +243,7 @@ describe("CoderAgentChatAction", () => { const parsedResult = ActionOutputsSchema.parse(result); expect(parsedResult.coderUsername).toBe(mockUser.username); expect(parsedResult.chatCreated).toBe(true); + expect(octokit.rest.issues.createComment).toHaveBeenCalled(); }); test("sends message to existing chat", async () => { @@ -246,6 +251,7 @@ describe("CoderAgentChatAction", () => { coderClient.mockCreateChatMessage.mockResolvedValue( mockChatMessageResponse, ); + mockOctokitCommentingHappyPath(octokit); const existingChatId = "990e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ @@ -272,11 +278,13 @@ describe("CoderAgentChatAction", () => { const parsedResult = ActionOutputsSchema.parse(result); expect(parsedResult.chatCreated).toBe(false); expect(parsedResult.chatId).toBe(existingChatId); + expect(octokit.rest.issues.createComment).toHaveBeenCalled(); }); test("creates chat with workspace-id", async () => { coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); + mockOctokitCommentingHappyPath(octokit); const workspaceId = "550e8400-e29b-41d4-a716-446655440000"; const inputs = createMockInputs({ @@ -296,6 +304,7 @@ describe("CoderAgentChatAction", () => { workspace_id: workspaceId, }), ); + expect(octokit.rest.issues.createComment).toHaveBeenCalled(); }); describe("commentOnIssue toggle", () => { @@ -319,15 +328,33 @@ describe("CoderAgentChatAction", () => { expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); }); + test("does not comment when commentOnIssue is false (existing chat path)", async () => { + coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockCreateChatMessage.mockResolvedValue( + mockChatMessageResponse, + ); + + const inputs = createMockInputs({ + githubUserID: 12345, + existingChatId: "990e8400-e29b-41d4-a716-446655440000", + commentOnIssue: false, + }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + await action.run(); + + expect(octokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.rest.issues.createComment).not.toHaveBeenCalled(); + }); + test("comments when commentOnIssue is true", async () => { coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockResolvedValue(mockChat); - octokit.rest.issues.listComments.mockResolvedValue({ - data: [], - } as ReturnType); - octokit.rest.issues.createComment.mockResolvedValue( - {} as ReturnType, - ); + mockOctokitCommentingHappyPath(octokit); const inputs = createMockInputs({ githubUserID: 12345, @@ -360,11 +387,30 @@ describe("CoderAgentChatAction", () => { inputs, ); - expect(action.run()).rejects.toThrow( + await expect(action.run()).rejects.toThrow( "No Coder user found with GitHub user ID 12345", ); }); + test("throws error when sending message to existing chat fails", async () => { + coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); + coderClient.mockCreateChatMessage.mockRejectedValue( + new Error("Chat not found"), + ); + + const inputs = createMockInputs({ + githubUserID: 12345, + existingChatId: "990e8400-e29b-41d4-a716-446655440000", + }); + const action = new CoderAgentChatAction( + coderClient, + octokit as unknown as Octokit, + inputs, + ); + + await expect(action.run()).rejects.toThrow("Chat not found"); + }); + test("throws error when chat creation fails", async () => { coderClient.mockGetCoderUserByGithubID.mockResolvedValue(mockUser); coderClient.mockCreateChat.mockRejectedValue( @@ -378,7 +424,7 @@ describe("CoderAgentChatAction", () => { inputs, ); - expect(action.run()).rejects.toThrow("Failed to create chat"); + await expect(action.run()).rejects.toThrow("Failed to create chat"); }); }); }); diff --git a/src/coder-client.test.ts b/src/coder-client.test.ts index 780ee85..9d649ef 100644 --- a/src/coder-client.test.ts +++ b/src/coder-client.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, beforeEach, mock } from "bun:test"; +import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; import { RealCoderClient, CoderAPIError } from "./coder-client"; import { mockUser, @@ -14,14 +14,20 @@ import { describe("CoderClient", () => { let client: RealCoderClient; let mockFetch: ReturnType; + let originalFetch: typeof fetch; beforeEach(() => { + originalFetch = global.fetch; const mockInputs = createMockInputs(); client = new RealCoderClient(mockInputs.coderURL, mockInputs.coderToken); mockFetch = mock(() => Promise.resolve(createMockResponse([]))); global.fetch = mockFetch as unknown as typeof fetch; }); + afterEach(() => { + global.fetch = originalFetch; + }); + describe("getCoderUserByGitHubId", () => { test("returns the user when found", async () => { mockFetch.mockResolvedValue(createMockResponse(mockUserList)); @@ -42,14 +48,14 @@ describe("CoderClient", () => { test("throws when multiple users found", async () => { mockFetch.mockResolvedValue(createMockResponse(mockUserListDuplicate)); - expect( + await expect( client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), ).rejects.toThrow(CoderAPIError); }); test("throws when no user found", async () => { mockFetch.mockResolvedValue(createMockResponse(mockUserListEmpty)); - expect( + await expect( client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), ).rejects.toThrow(CoderAPIError); }); @@ -61,13 +67,13 @@ describe("CoderClient", () => { { ok: false, status: 401, statusText: "Unauthorized" }, ), ); - expect( + await expect( client.getCoderUserByGitHubId(mockUser.github_com_user_id ?? 0), ).rejects.toThrow(CoderAPIError); }); test("throws when GitHub user ID is 0", async () => { - expect(client.getCoderUserByGitHubId(0)).rejects.toThrow( + await expect(client.getCoderUserByGitHubId(0)).rejects.toThrow( "GitHub user ID cannot be 0", ); }); @@ -132,7 +138,7 @@ describe("CoderClient", () => { { ok: false, status: 404, statusText: "Not Found" }, ), ); - expect( + await expect( client.createChatMessage(mockChat.id, { content: [{ type: "text", text: "Test" }], }), @@ -140,6 +146,21 @@ describe("CoderClient", () => { }); }); + describe("listChats", () => { + test("returns chat list", async () => { + mockFetch.mockResolvedValue(createMockResponse([mockChat])); + const result = await client.listChats(); + expect(result).toHaveLength(1); + expect(result[0].id).toBe(mockChat.id); + }); + + test("returns empty list", async () => { + mockFetch.mockResolvedValue(createMockResponse([])); + const result = await client.listChats(); + expect(result).toHaveLength(0); + }); + }); + describe("getChat", () => { test("returns chat when found", async () => { mockFetch.mockResolvedValue(createMockResponse(mockChat)); @@ -156,4 +177,12 @@ describe("CoderClient", () => { ); }); }); + + describe("getCoderUserByGitHubId edge cases", () => { + test("throws when GitHub user ID is undefined", async () => { + await expect(client.getCoderUserByGitHubId(undefined)).rejects.toThrow( + "GitHub user ID cannot be undefined", + ); + }); + }); }); diff --git a/src/schemas.test.ts b/src/schemas.test.ts index e4da93f..588bdfd 100644 --- a/src/schemas.test.ts +++ b/src/schemas.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { type ActionInputs, ActionInputsSchema } from "./schemas"; +import { + type ActionInputs, + ActionInputsSchema, + ActionOutputsSchema, +} from "./schemas"; const actionInputValid: ActionInputs = { coderURL: "https://coder.test", @@ -163,3 +167,47 @@ describe("ActionInputsSchema", () => { }); }); }); + +describe("ActionOutputsSchema", () => { + test("accepts valid outputs", () => { + const result = ActionOutputsSchema.parse({ + coderUsername: "testuser", + chatId: "550e8400-e29b-41d4-a716-446655440000", + chatUrl: "https://coder.test/chats/550e8400-e29b-41d4-a716-446655440000", + chatCreated: true, + }); + expect(result.coderUsername).toBe("testuser"); + }); + + test("rejects missing chatId", () => { + expect(() => + ActionOutputsSchema.parse({ + coderUsername: "testuser", + chatUrl: "https://coder.test/chats/abc", + chatCreated: true, + }), + ).toThrow(); + }); + + test("rejects invalid chatUrl", () => { + expect(() => + ActionOutputsSchema.parse({ + coderUsername: "testuser", + chatId: "550e8400-e29b-41d4-a716-446655440000", + chatUrl: "not-a-url", + chatCreated: true, + }), + ).toThrow(); + }); + + test("rejects non-boolean chatCreated", () => { + expect(() => + ActionOutputsSchema.parse({ + coderUsername: "testuser", + chatId: "550e8400-e29b-41d4-a716-446655440000", + chatUrl: "https://coder.test/chats/abc", + chatCreated: "yes", + }), + ).toThrow(); + }); +}); diff --git a/src/test-helpers.ts b/src/test-helpers.ts index f5686fa..371cd0b 100644 --- a/src/test-helpers.ts +++ b/src/test-helpers.ts @@ -126,6 +126,17 @@ export function createMockOctokit() { }; } +/** + * Set up Octokit mocks for the happy path of commenting on an issue + * (no existing comment, so a new one is created). + */ +export function mockOctokitCommentingHappyPath( + octokit: ReturnType, +) { + octokit.rest.issues.listComments.mockResolvedValue({ data: [] }); + octokit.rest.issues.createComment.mockResolvedValue({}); +} + /** * Create mock fetch response */ From bead3901fc5369e3fb04ae8e4c8b0c238330f058 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 20 Mar 2026 10:05:38 +0000 Subject: [PATCH 2/3] chore: rebuild dist/index.js --- dist/index.js | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/dist/index.js b/dist/index.js index 66e0d14..54bb743 100644 --- a/dist/index.js +++ b/dist/index.js @@ -3,25 +3,43 @@ var __getProtoOf = Object.getPrototypeOf; var __defProp = Object.defineProperty; var __getOwnPropNames = Object.getOwnPropertyNames; var __hasOwnProp = Object.prototype.hasOwnProperty; +function __accessProp(key) { + return this[key]; +} +var __toESMCache_node; +var __toESMCache_esm; var __toESM = (mod, isNodeMode, target) => { + var canCache = mod != null && typeof mod === "object"; + if (canCache) { + var cache = isNodeMode ? __toESMCache_node ??= new WeakMap : __toESMCache_esm ??= new WeakMap; + var cached = cache.get(mod); + if (cached) + return cached; + } target = mod != null ? __create(__getProtoOf(mod)) : {}; const to = isNodeMode || !mod || !mod.__esModule ? __defProp(target, "default", { value: mod, enumerable: true }) : target; for (let key of __getOwnPropNames(mod)) if (!__hasOwnProp.call(to, key)) __defProp(to, key, { - get: () => mod[key], + get: __accessProp.bind(mod, key), enumerable: true }); + if (canCache) + cache.set(mod, to); return to; }; var __commonJS = (cb, mod) => () => (mod || cb((mod = { exports: {} }).exports, mod), mod.exports); +var __returnValue = (v) => v; +function __exportSetter(name, newValue) { + this[name] = __returnValue.bind(null, newValue); +} var __export = (target, all) => { for (var name in all) __defProp(target, name, { get: all[name], enumerable: true, configurable: true, - set: (newValue) => all[name] = () => newValue + set: __exportSetter.bind(all, name) }); }; @@ -3447,7 +3465,7 @@ var require_constants2 = __commonJS((exports2, module2) => { } })(); var channel; - var structuredClone = globalThis.structuredClone ?? function structuredClone(value, options = undefined) { + var structuredClone = globalThis.structuredClone ?? function structuredClone2(value, options = undefined) { if (arguments.length === 0) { throw new TypeError("missing argument"); } @@ -16372,7 +16390,7 @@ var require_undici = __commonJS((exports2, module2) => { module2.exports.getGlobalDispatcher = getGlobalDispatcher; if (util.nodeMajor > 16 || util.nodeMajor === 16 && util.nodeMinor >= 8) { let fetchImpl = null; - module2.exports.fetch = async function fetch(resource) { + module2.exports.fetch = async function fetch2(resource) { if (!fetchImpl) { fetchImpl = require_fetch().fetch; } @@ -22708,11 +22726,11 @@ var require_github = __commonJS((exports2) => { }); // src/index.ts -var core2 = __toESM(require_core()); -var github = __toESM(require_github()); +var core2 = __toESM(require_core(), 1); +var github = __toESM(require_github(), 1); // src/action.ts -var core = __toESM(require_core()); +var core = __toESM(require_core(), 1); class CoderAgentChatAction { coder; From 4cfb366a59cffa18970d8c2e9249e6cba691629d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 20 Mar 2026 10:27:09 +0000 Subject: [PATCH 3/3] fix: stop throwing strings like a barbarian, use CoderAPIError consistently --- dist/index.js | 2 +- src/coder-client.test.ts | 2 +- src/coder-client.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dist/index.js b/dist/index.js index 54bb743..8d54e46 100644 --- a/dist/index.js +++ b/dist/index.js @@ -26852,7 +26852,7 @@ class RealCoderClient { throw new CoderAPIError("GitHub user ID cannot be undefined", 400); } if (githubUserId === 0) { - throw "GitHub user ID cannot be 0"; + throw new CoderAPIError("GitHub user ID cannot be 0", 400); } const endpoint = `/api/v2/users?q=${encodeURIComponent(`github_com_user_id:${githubUserId}`)}`; const response = await this.request(endpoint); diff --git a/src/coder-client.test.ts b/src/coder-client.test.ts index 9d649ef..5b16bf1 100644 --- a/src/coder-client.test.ts +++ b/src/coder-client.test.ts @@ -74,7 +74,7 @@ describe("CoderClient", () => { test("throws when GitHub user ID is 0", async () => { await expect(client.getCoderUserByGitHubId(0)).rejects.toThrow( - "GitHub user ID cannot be 0", + CoderAPIError, ); }); }); diff --git a/src/coder-client.ts b/src/coder-client.ts index 0a6b949..1332d03 100644 --- a/src/coder-client.ts +++ b/src/coder-client.ts @@ -66,7 +66,7 @@ export class RealCoderClient implements CoderClient { throw new CoderAPIError("GitHub user ID cannot be undefined", 400); } if (githubUserId === 0) { - throw "GitHub user ID cannot be 0"; + throw new CoderAPIError("GitHub user ID cannot be 0", 400); } const endpoint = `/api/v2/users?q=${encodeURIComponent(`github_com_user_id:${githubUserId}`)}`; const response = await this.request(endpoint);