test(KB-316): add comprehensive test coverage for dashboard services

- Add file service tests (680 lines) covering file operations and error handling
- Extend GitHub polling service tests with GitHubPollingService coverage
- Extend rate-limit tests with additional boundary and edge cases
- Total of 1405 new test lines across 3 test files
This commit is contained in:
gsxdsm
2026-03-31 12:01:55 -07:00
parent ca31314be0
commit a9b0cb55f9
3 changed files with 1405 additions and 3 deletions

View File

@@ -0,0 +1,680 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import {
FileServiceError,
listFiles,
readFile,
writeFile,
listProjectFiles,
readProjectFile,
writeProjectFile,
listWorkspaceFiles,
readWorkspaceFile,
writeWorkspaceFile,
MAX_FILE_SIZE,
} from "../file-service.js";
import type { TaskStore } from "@kb/core";
// Mock node:fs/promises
const mockReaddir = vi.fn();
const mockReadFile = vi.fn();
const mockWriteFile = vi.fn();
const mockStat = vi.fn();
vi.mock("node:fs/promises", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs/promises")>();
return {
...actual,
readdir: (...args: any[]) => mockReaddir(...args),
readFile: (...args: any[]) => mockReadFile(...args),
writeFile: (...args: any[]) => mockWriteFile(...args),
stat: (...args: any[]) => mockStat(...args),
};
});
// Mock node:fs
const mockExistsSync = vi.fn();
vi.mock("node:fs", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:fs")>();
return {
...actual,
existsSync: (...args: any[]) => mockExistsSync(...args),
};
});
describe("FileServiceError", () => {
it("constructor sets code and name correctly", () => {
const error = new FileServiceError("Test message", "ETEST");
expect(error.message).toBe("Test message");
expect(error.code).toBe("ETEST");
expect(error.name).toBe("FileServiceError");
});
it("is an instance of Error", () => {
const error = new FileServiceError("Test", "ETEST");
expect(error).toBeInstanceOf(Error);
});
});
describe("MAX_FILE_SIZE", () => {
it("is 1MB (1024 * 1024 bytes)", () => {
expect(MAX_FILE_SIZE).toBe(1024 * 1024);
expect(MAX_FILE_SIZE).toBe(1048576);
});
});
describe("path traversal protection", () => {
const mockGetTask = vi.fn();
const mockGetRootDir = vi.fn();
const mockStore = {
getTask: mockGetTask,
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetTask.mockReset();
mockGetRootDir.mockReset();
mockStat.mockReset();
mockReaddir.mockReset();
mockExistsSync.mockReset();
});
describe("via listProjectFiles", () => {
it("rejects path traversal attacks (../)", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(listProjectFiles(mockStore, "../secret.txt")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "../secret.txt")).rejects.toThrow("Path traversal detected");
});
it("rejects absolute paths", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(listProjectFiles(mockStore, "/etc/passwd")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "/etc/passwd")).rejects.toThrow("Absolute paths not allowed");
});
it("rejects paths with null bytes", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(listProjectFiles(mockStore, "file\0.txt")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "file\0.txt")).rejects.toThrow("Invalid characters");
});
it("rejects URL-encoded path traversal", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(listProjectFiles(mockStore, "%2e%2e%2fsecret.txt")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "%2e%2e%2fsecret.txt")).rejects.toThrow("Path traversal detected");
});
});
describe("via readProjectFile", () => {
it("rejects path traversal attacks", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(readProjectFile(mockStore, "../.env")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "../.env")).rejects.toThrow("Path traversal detected");
});
it("rejects absolute paths", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(readProjectFile(mockStore, "/etc/passwd")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "/etc/passwd")).rejects.toThrow("Absolute paths not allowed");
});
});
describe("via writeProjectFile", () => {
it("rejects path traversal attacks", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(writeProjectFile(mockStore, "../.env", "evil")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "../.env", "evil")).rejects.toThrow("Path traversal detected");
});
it("rejects null bytes in path", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(writeProjectFile(mockStore, "file\0.txt", "content")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "file\0.txt", "content")).rejects.toThrow("Invalid characters");
});
it("rejects absolute paths", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(writeProjectFile(mockStore, "/etc/crontab", "evil")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "/etc/crontab", "evil")).rejects.toThrow("Absolute paths not allowed");
});
});
describe("via listFiles (task)", () => {
it("rejects path traversal in task context", async () => {
mockGetRootDir.mockReturnValue("/project");
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
await expect(listFiles(mockStore, "KB-123", "../other-task")).rejects.toThrow(FileServiceError);
await expect(listFiles(mockStore, "KB-123", "../other-task")).rejects.toThrow("Path traversal detected");
});
it("rejects absolute paths in task context", async () => {
mockGetRootDir.mockReturnValue("/project");
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
await expect(listFiles(mockStore, "KB-123", "/etc/passwd")).rejects.toThrow(FileServiceError);
});
});
describe("via readFile (task)", () => {
it("rejects path traversal when reading task files", async () => {
mockGetRootDir.mockReturnValue("/project");
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
await expect(readFile(mockStore, "KB-123", "../../secret.txt")).rejects.toThrow(FileServiceError);
await expect(readFile(mockStore, "KB-123", "../../secret.txt")).rejects.toThrow("Path traversal detected");
});
});
describe("via writeFile (task)", () => {
it("rejects path traversal when writing task files", async () => {
mockGetRootDir.mockReturnValue("/project");
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
await expect(writeFile(mockStore, "KB-123", "../../outside.txt", "data")).rejects.toThrow(FileServiceError);
});
});
describe("via workspace operations", () => {
it("rejects path traversal in workspace file listing", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(listWorkspaceFiles(mockStore, "project", "../../outside")).rejects.toThrow(FileServiceError);
});
it("rejects path traversal in workspace file read", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(readWorkspaceFile(mockStore, "project", "../.env")).rejects.toThrow(FileServiceError);
});
it("rejects path traversal in workspace file write", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(writeWorkspaceFile(mockStore, "project", "../.env", "data")).rejects.toThrow(FileServiceError);
});
it("rejects absolute paths in workspace file read", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(readWorkspaceFile(mockStore, "project", "/etc/passwd")).rejects.toThrow(FileServiceError);
});
});
describe("complex path traversal patterns", () => {
it("rejects nested path traversal", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(listProjectFiles(mockStore, "foo/../../secret")).rejects.toThrow(FileServiceError);
});
it("rejects parent directory at root", async () => {
mockGetRootDir.mockReturnValue("/project");
await expect(listProjectFiles(mockStore, "..")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "../")).rejects.toThrow(FileServiceError);
});
it("allows valid relative paths with dots", async () => {
mockGetRootDir.mockReturnValue("/project");
// Need successful stat for this test to pass validation
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
});
mockReaddir.mockResolvedValue([]);
// This should NOT throw
await expect(listProjectFiles(mockStore, "./src")).resolves.not.toThrow();
});
it("allows paths containing single dots in middle", async () => {
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
});
mockReaddir.mockResolvedValue([]);
await expect(listProjectFiles(mockStore, "src/./components")).resolves.not.toThrow();
});
});
});
describe("listProjectFiles", () => {
const mockGetRootDir = vi.fn();
const mockStore = {
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetRootDir.mockReset();
mockStat.mockReset();
mockReaddir.mockReset();
});
it("throws FileServiceError with ENOENT for missing directory", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockRejectedValue({ code: "ENOENT" });
await expect(listProjectFiles(mockStore, "missing")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "missing")).rejects.toThrow("Directory not found");
});
it("throws FileServiceError with ENOTDIR for non-directory", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockResolvedValue({
isDirectory: () => false,
isFile: () => true,
});
await expect(listProjectFiles(mockStore, "file.txt")).rejects.toThrow(FileServiceError);
await expect(listProjectFiles(mockStore, "file.txt")).rejects.toThrow("Not a directory");
});
});
describe("readProjectFile", () => {
const mockGetRootDir = vi.fn();
const mockStore = {
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetRootDir.mockReset();
mockStat.mockReset();
});
it("enforces max file size limit (1MB)", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockResolvedValue({
isFile: () => true,
size: MAX_FILE_SIZE + 1,
mtime: new Date(),
});
await expect(readProjectFile(mockStore, "large.bin")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "large.bin")).rejects.toThrow("File too large");
});
it("throws FileServiceError with ENOENT for missing file", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockRejectedValue({ code: "ENOENT" });
await expect(readProjectFile(mockStore, "missing.txt")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "missing.txt")).rejects.toThrow("File not found");
});
it("throws FileServiceError when path is not a file", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockResolvedValue({
isFile: () => false,
isDirectory: () => true,
});
await expect(readProjectFile(mockStore, "src")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "src")).rejects.toThrow("Not a file");
});
it("requires file path", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(readProjectFile(mockStore, "")).rejects.toThrow(FileServiceError);
await expect(readProjectFile(mockStore, "")).rejects.toThrow("File path is required");
});
});
describe("writeProjectFile", () => {
const mockGetRootDir = vi.fn();
const mockStore = {
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetRootDir.mockReset();
mockStat.mockReset();
mockWriteFile.mockReset();
});
it("prevents writing to directories", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
});
await expect(writeProjectFile(mockStore, "src", "content")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "src", "content")).rejects.toThrow("Cannot write to directory");
});
it("validates parent directory exists", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat
.mockRejectedValueOnce({ code: "ENOENT" }) // File doesn't exist
.mockRejectedValueOnce({ code: "ENOENT" }); // Parent doesn't exist
await expect(writeProjectFile(mockStore, "missing/file.txt", "content")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "missing/file.txt", "content")).rejects.toThrow("Parent directory does not exist");
});
it("throws when parent is not a directory", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat
.mockRejectedValueOnce({ code: "ENOENT" }) // File doesn't exist
.mockResolvedValueOnce({
isDirectory: () => false,
isFile: () => true,
}); // Parent is a file
await expect(writeProjectFile(mockStore, "file.txt/sub.txt", "content")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "file.txt/sub.txt", "content")).rejects.toThrow("Parent is not a directory");
});
it("requires file path", async () => {
mockGetRootDir.mockReturnValue("/test/project");
await expect(writeProjectFile(mockStore, "", "content")).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "", "content")).rejects.toThrow("File path is required");
});
it("enforces max content size", async () => {
mockGetRootDir.mockReturnValue("/test/project");
const largeContent = "x".repeat(MAX_FILE_SIZE + 1);
await expect(writeProjectFile(mockStore, "large.txt", largeContent)).rejects.toThrow(FileServiceError);
await expect(writeProjectFile(mockStore, "large.txt", largeContent)).rejects.toThrow("Content too large");
});
});
describe("task file operations", () => {
const mockGetTask = vi.fn();
const mockGetRootDir = vi.fn();
const mockStore = {
getTask: mockGetTask,
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetTask.mockReset();
mockGetRootDir.mockReset();
mockStat.mockReset();
mockReaddir.mockReset();
mockReadFile.mockReset();
mockWriteFile.mockReset();
mockExistsSync.mockReset();
});
describe("getTaskBasePath", () => {
it("returns worktree path if it exists", async () => {
const worktreePath = "/worktrees/kb-123";
mockGetTask.mockResolvedValue({
id: "KB-123",
worktree: worktreePath,
});
mockExistsSync.mockReturnValue(true);
mockStat.mockResolvedValue({
isFile: () => true,
size: 100,
mtime: new Date("2024-01-01"),
});
mockReadFile.mockResolvedValue("Task content");
const result = await readFile(mockStore, "KB-123", "PROMPT.md");
expect(mockReadFile).toHaveBeenCalledWith(
"/worktrees/kb-123/PROMPT.md",
"utf-8",
);
expect(result.content).toBe("Task content");
});
it("falls back to task directory when worktree doesn't exist", async () => {
mockGetTask.mockResolvedValue({
id: "KB-123",
worktree: "/missing/worktree",
});
mockGetRootDir.mockReturnValue("/project");
mockExistsSync.mockReturnValue(false);
mockStat.mockResolvedValue({
isFile: () => true,
size: 100,
mtime: new Date("2024-01-01"),
});
mockReadFile.mockResolvedValue("Task content");
await readFile(mockStore, "KB-123", "PROMPT.md");
expect(mockReadFile).toHaveBeenCalledWith(
"/project/.kb/tasks/KB-123/PROMPT.md",
"utf-8",
);
});
it("falls back to task directory when worktree is undefined", async () => {
mockGetTask.mockResolvedValue({
id: "KB-123",
worktree: undefined,
});
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isFile: () => true,
size: 100,
mtime: new Date("2024-01-01"),
});
mockReadFile.mockResolvedValue("Task content");
await readFile(mockStore, "KB-123", "PROMPT.md");
expect(mockReadFile).toHaveBeenCalledWith(
"/project/.kb/tasks/KB-123/PROMPT.md",
"utf-8",
);
});
it("throws ENOTASK for missing task (ENOENT)", async () => {
mockGetTask.mockRejectedValue({ code: "ENOENT" });
await expect(readFile(mockStore, "KB-999", "file.txt")).rejects.toThrow(FileServiceError);
});
it("throws ENOTASK for task not found error message", async () => {
mockGetTask.mockRejectedValue(new Error("Task not found"));
await expect(readFile(mockStore, "KB-999", "file.txt")).rejects.toThrow(FileServiceError);
});
});
});
describe("workspace operations", () => {
const mockGetTask = vi.fn();
const mockGetRootDir = vi.fn();
const mockStore = {
getTask: mockGetTask,
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetTask.mockReset();
mockGetRootDir.mockReset();
mockStat.mockReset();
mockReaddir.mockReset();
mockReadFile.mockReset();
mockWriteFile.mockReset();
mockExistsSync.mockReset();
});
describe("listWorkspaceFiles", () => {
it('"project" workspace resolves to project root', async () => {
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
});
mockReaddir.mockResolvedValue([
{ name: "src", isDirectory: () => true, isFile: () => false },
]);
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
size: 0,
mtime: new Date(),
});
const result = await listWorkspaceFiles(mockStore, "project");
expect(result.entries).toHaveLength(1);
expect(result.entries[0].name).toBe("src");
});
it("task ID workspace resolves to task path", async () => {
mockGetTask.mockResolvedValue({ id: "KB-456", worktree: undefined });
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isDirectory: () => true,
isFile: () => false,
});
mockReaddir.mockResolvedValue([
{ name: "PROMPT.md", isDirectory: () => false, isFile: () => true },
]);
mockStat.mockResolvedValue({
isDirectory: () => false,
isFile: () => true,
size: 100,
mtime: new Date(),
});
const result = await listWorkspaceFiles(mockStore, "KB-456");
expect(result.entries).toHaveLength(1);
expect(result.entries[0].name).toBe("PROMPT.md");
});
});
describe("readWorkspaceFile", () => {
it("reads file from project workspace", async () => {
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isFile: () => true,
size: 50,
mtime: new Date("2024-01-01"),
});
mockReadFile.mockResolvedValue("File contents");
const result = await readWorkspaceFile(mockStore, "project", "README.md");
expect(result.content).toBe("File contents");
expect(mockReadFile).toHaveBeenCalledWith(
"/project/README.md",
"utf-8",
);
});
it("reads file from task workspace", async () => {
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
mockGetRootDir.mockReturnValue("/project");
mockStat.mockResolvedValue({
isFile: () => true,
size: 200,
mtime: new Date("2024-02-01"),
});
mockReadFile.mockResolvedValue("Task description");
const result = await readWorkspaceFile(mockStore, "KB-123", "PROMPT.md");
expect(result.content).toBe("Task description");
expect(mockReadFile).toHaveBeenCalledWith(
"/project/.kb/tasks/KB-123/PROMPT.md",
"utf-8",
);
});
});
describe("writeWorkspaceFile", () => {
it("writes file to project workspace", async () => {
mockGetRootDir.mockReturnValue("/project");
mockStat
.mockRejectedValueOnce({ code: "ENOENT" })
.mockResolvedValueOnce({ isDirectory: () => true });
mockWriteFile.mockResolvedValue(undefined);
mockStat.mockResolvedValue({
size: 20,
mtime: new Date("2024-03-01"),
});
const result = await writeWorkspaceFile(mockStore, "project", "notes.txt", "My notes");
expect(result.success).toBe(true);
expect(mockWriteFile).toHaveBeenCalledWith(
"/project/notes.txt",
"My notes",
"utf-8",
);
});
it("writes file to task workspace", async () => {
mockGetTask.mockResolvedValue({ id: "KB-123", worktree: undefined });
mockGetRootDir.mockReturnValue("/project");
mockStat
.mockRejectedValueOnce({ code: "ENOENT" })
.mockResolvedValueOnce({ isDirectory: () => true });
mockWriteFile.mockResolvedValue(undefined);
mockStat.mockResolvedValue({
size: 30,
mtime: new Date("2024-04-01"),
});
const result = await writeWorkspaceFile(mockStore, "KB-123", "output.txt", "Task output");
expect(result.success).toBe(true);
expect(mockWriteFile).toHaveBeenCalledWith(
"/project/.kb/tasks/KB-123/output.txt",
"Task output",
"utf-8",
);
});
});
});
describe("URL-encoded characters handling", () => {
const mockGetRootDir = vi.fn();
const mockStore = {
getRootDir: mockGetRootDir,
} as unknown as TaskStore;
beforeEach(() => {
mockGetRootDir.mockReset();
mockStat.mockReset();
});
it("decodes URL-encoded characters safely in file paths", async () => {
mockGetRootDir.mockReturnValue("/test/project");
mockStat.mockResolvedValue({
isFile: () => true,
size: 100,
mtime: new Date("2024-01-01"),
});
mockReadFile.mockResolvedValue("Content");
await readProjectFile(mockStore, "file%20name.txt");
// Should decode %20 to space and look for the file
expect(mockReadFile).toHaveBeenCalledWith(
"/test/project/file name.txt",
"utf-8",
);
});
});

View File

@@ -1,5 +1,15 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { GitHubRateLimiter } from "../github-poll.js";
import { GitHubRateLimiter, GitHubPollingService, githubPoller, githubRateLimiter } from "../github-poll.js";
import type { TaskStore } from "@kb/core";
// Mock the GitHubClient
const mockGetBadgeStatusesBatch = vi.fn();
vi.mock("../github.js", () => ({
GitHubClient: vi.fn().mockImplementation(() => ({
getBadgeStatusesBatch: (...args: any[]) => mockGetBadgeStatusesBatch(...args),
})),
}));
describe("GitHubRateLimiter", () => {
beforeEach(() => {
@@ -84,3 +94,596 @@ describe("GitHubRateLimiter", () => {
expect(limiter.canMakeRequest("owner/repo")).toBe(false);
});
});
describe("GitHubPollingService", () => {
let service: GitHubPollingService;
let mockStore: TaskStore;
let mockUpdatePrInfo: vi.Mock;
let mockUpdateIssueInfo: vi.Mock;
let mockGetTask: vi.Mock;
beforeEach(() => {
vi.useFakeTimers();
mockUpdatePrInfo = vi.fn();
mockUpdateIssueInfo = vi.fn();
mockGetTask = vi.fn();
mockStore = {
getTask: mockGetTask,
updatePrInfo: mockUpdatePrInfo,
updateIssueInfo: mockUpdateIssueInfo,
} as unknown as TaskStore;
service = new GitHubPollingService({
store: mockStore,
token: "test-token",
pollingIntervalMs: 60_000,
});
mockGetBadgeStatusesBatch.mockReset();
});
afterEach(() => {
service.stop();
service.reset();
vi.useRealTimers();
vi.restoreAllMocks();
});
describe("configure", () => {
it("updates store, token, and polling interval", () => {
const newStore = { getTask: vi.fn() } as unknown as TaskStore;
service.configure({
store: newStore,
token: "new-token",
pollingIntervalMs: 30_000,
});
// Verify by checking the service behavior uses new config
expect(service.getWatchedTaskIds()).toEqual([]);
});
it("restarts timer when interval changes while running", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.start();
expect(service["timer"]).not.toBeNull();
const originalTimer = service["timer"];
service.configure({ pollingIntervalMs: 30_000 });
// Timer should have been restarted with new interval
expect(service["timer"]).not.toBe(originalTimer);
});
it("does not restart timer if not running", () => {
service.configure({ pollingIntervalMs: 30_000 });
expect(service["timer"]).toBeNull();
});
});
describe("start/stop", () => {
it("begins polling when watches exist", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.start();
expect(service["enabled"]).toBe(true);
expect(service["timer"]).not.toBeNull();
});
it("does nothing when no watches", () => {
service.start();
expect(service["timer"]).toBeNull();
});
it("clears timer on stop", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.start();
expect(service["timer"]).not.toBeNull();
service.stop();
expect(service["timer"]).toBeNull();
expect(service["enabled"]).toBe(false);
});
it("multiple start calls are safe", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.start();
const timer1 = service["timer"];
service.start();
const timer2 = service["timer"];
expect(timer1).toBe(timer2);
});
it("stop is safe when not running", () => {
expect(() => service.stop()).not.toThrow();
});
});
describe("watchTask", () => {
it("adds watch for task", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
const watch = service.getWatch("KB-001");
expect(watch).toBeDefined();
expect(watch?.pr).toEqual({
taskId: "KB-001",
type: "pr",
owner: "owner",
repo: "repo",
number: 1,
});
});
it("replaces existing watch of same type", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-001", "pr", "owner", "repo", 2);
const watch = service.getWatch("KB-001");
expect(watch?.pr?.number).toBe(2);
});
it("keeps other watch type when replacing", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-001", "issue", "owner", "repo", 10);
const watch = service.getWatch("KB-001");
expect(watch?.pr?.number).toBe(1);
expect(watch?.issue?.number).toBe(10);
});
});
describe("replaceTaskWatches", () => {
it("handles multiple watch types", () => {
service.replaceTaskWatches("KB-001", [
{ taskId: "KB-001", type: "pr", owner: "owner", repo: "repo", number: 1 },
{ taskId: "KB-001", type: "issue", owner: "owner", repo: "repo", number: 10 },
]);
const watch = service.getWatch("KB-001");
expect(watch?.pr?.number).toBe(1);
expect(watch?.issue?.number).toBe(10);
});
it("unwatches when empty array", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.replaceTaskWatches("KB-001", []);
expect(service.getWatch("KB-001")).toBeUndefined();
});
it("filters invalid watches", () => {
service.replaceTaskWatches("KB-001", [
{ taskId: "KB-001", type: "pr", owner: "", repo: "repo", number: 1 }, // invalid - empty owner
{ taskId: "KB-001", type: "issue", owner: "owner", repo: "repo", number: 10 }, // valid
]);
const watch = service.getWatch("KB-001");
expect(watch?.pr).toBeUndefined();
expect(watch?.issue?.number).toBe(10);
});
});
describe("unwatchTask", () => {
it("removes all watches for task", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-001", "issue", "owner", "repo", 10);
service.unwatchTask("KB-001");
expect(service.getWatch("KB-001")).toBeUndefined();
});
it("stops polling when no watches remain", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.start();
expect(service["timer"]).not.toBeNull();
service.unwatchTask("KB-001");
expect(service["timer"]).toBeNull();
});
});
describe("unwatchTaskType", () => {
it("removes specific type only", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-001", "issue", "owner", "repo", 10);
service.unwatchTaskType("KB-001", "pr");
const watch = service.getWatch("KB-001");
expect(watch?.pr).toBeUndefined();
expect(watch?.issue?.number).toBe(10);
});
it("unwatches task if no types remain", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.unwatchTaskType("KB-001", "pr");
expect(service.getWatch("KB-001")).toBeUndefined();
});
});
describe("reset", () => {
it("clears all watches and stops", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-002", "issue", "owner", "repo", 10);
service.start();
service.reset();
expect(service.getWatchedTaskIds()).toEqual([]);
expect(service["timer"]).toBeNull();
});
});
describe("getWatchedTaskIds", () => {
it("returns all watched task IDs", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
service.watchTask("KB-002", "issue", "owner", "repo", 10);
const ids = service.getWatchedTaskIds();
expect(ids).toContain("KB-001");
expect(ids).toContain("KB-002");
expect(ids).toHaveLength(2);
});
});
describe("getWatch", () => {
it("returns watch set for task", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
const watch = service.getWatch("KB-001");
expect(watch?.pr?.owner).toBe("owner");
});
it("returns undefined for unwatched task", () => {
expect(service.getWatch("KB-999")).toBeUndefined();
});
});
describe("getLastCheckedAt", () => {
it("returns timestamp for type", async () => {
// Setup task with PR badge
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
const checkedAt = service.getLastCheckedAt("KB-001", "pr");
expect(checkedAt).toBeDefined();
expect(new Date(checkedAt!).getTime()).toBeGreaterThan(0);
});
it("returns undefined for unwatched type", () => {
service.watchTask("KB-001", "pr", "owner", "repo", 1);
expect(service.getLastCheckedAt("KB-001", "issue")).toBeUndefined();
});
});
describe("pollOnce", () => {
it("batches requests by repo", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
// Should batch by repo
expect(mockGetBadgeStatusesBatch).toHaveBeenCalledWith(
"owner",
"repo",
expect.arrayContaining([expect.objectContaining({ alias: "pr_1", type: "pr", number: 1 })])
);
});
it("applies rate limiting per repo", async () => {
// Create a custom rate limiter for testing
const rateLimiter = new GitHubRateLimiter({ maxRequests: 1, windowMs: 60000 });
service = new GitHubPollingService({
store: mockStore,
token: "test-token",
rateLimiter,
});
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
// First poll should work
await service.pollOnce();
expect(mockGetBadgeStatusesBatch).toHaveBeenCalledTimes(1);
// Second poll should be rate limited (same repo)
await service.pollOnce();
// Should not make another request due to rate limiting
expect(mockGetBadgeStatusesBatch).toHaveBeenCalledTimes(1);
});
it("handles missing tasks (ENOENT unwatches)", async () => {
mockGetTask.mockRejectedValue({ code: "ENOENT" });
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
// Task should be unwatched after ENOENT
expect(service.getWatch("KB-001")).toBeUndefined();
});
it("updates store when badge fields changed", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Old Title", headBranch: "feat", baseBranch: "main", commentCount: 0 },
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "New Title", headBranch: "feat", baseBranch: "main", commentCount: 1 },
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
expect(mockUpdatePrInfo).toHaveBeenCalledWith(
"KB-001",
expect.objectContaining({ title: "New Title", commentCount: 1 })
);
});
it("skips update when badge unchanged", async () => {
const prInfo = { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Same Title", headBranch: "feat", baseBranch: "main", commentCount: 0 };
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo,
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo,
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
expect(mockUpdatePrInfo).not.toHaveBeenCalled();
});
it("handles PR status normalization", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "open", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
});
// PR status can be "open", "closed", or "merged"
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: { url: "https://github.com/owner/repo/pull/1", number: 1, status: "merged", title: "Test", headBranch: "feat", baseBranch: "main", commentCount: 0 },
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
expect(mockUpdatePrInfo).toHaveBeenCalledWith(
"KB-001",
expect.objectContaining({ status: "merged" })
);
});
it("does nothing when store is not configured", async () => {
service = new GitHubPollingService({ token: "test-token" });
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await expect(service.pollOnce()).resolves.toBeUndefined();
});
it("does nothing when already polling", async () => {
service["isPolling"] = true;
await expect(service.pollOnce()).resolves.toBeUndefined();
});
it("does nothing when no watches", async () => {
await expect(service.pollOnce()).resolves.toBeUndefined();
});
});
describe("badge field comparison", () => {
it("detects PR badge changes (url, number, status, title, headBranch, baseBranch, commentCount, lastCommentAt)", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
prInfo: {
url: "https://github.com/owner/repo/pull/1",
number: 1,
status: "open",
title: "Test",
headBranch: "feat",
baseBranch: "main",
commentCount: 0,
lastCommentAt: undefined,
},
});
// Each field change should trigger update
const fieldChanges = [
{ field: "url", newValue: "https://github.com/owner/repo/pull/2" },
{ field: "number", newValue: 2 },
{ field: "status", newValue: "closed" },
{ field: "title", newValue: "New Title" },
{ field: "headBranch", newValue: "feature" },
{ field: "baseBranch", newValue: "develop" },
{ field: "commentCount", newValue: 1 },
{ field: "lastCommentAt", newValue: "2024-01-01T00:00:00Z" },
];
for (const change of fieldChanges) {
mockUpdatePrInfo.mockClear();
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: {
type: "pr",
prInfo: {
url: "https://github.com/owner/repo/pull/1",
number: 1,
status: "open",
title: "Test",
headBranch: "feat",
baseBranch: "main",
commentCount: 0,
[change.field]: change.newValue,
},
},
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
expect(mockUpdatePrInfo).toHaveBeenCalled();
}
});
it("detects issue badge changes (url, number, state, title, stateReason)", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
issueInfo: {
url: "https://github.com/owner/repo/issues/1",
number: 1,
state: "open",
title: "Test Issue",
stateReason: undefined,
},
});
const fieldChanges = [
{ field: "url", newValue: "https://github.com/owner/repo/issues/2" },
{ field: "number", newValue: 2 },
{ field: "state", newValue: "closed" },
{ field: "title", newValue: "New Issue Title" },
{ field: "stateReason", newValue: "completed" },
];
for (const change of fieldChanges) {
mockUpdateIssueInfo.mockClear();
mockGetBadgeStatusesBatch.mockResolvedValue({
issue_1: {
type: "issue",
issueInfo: {
url: "https://github.com/owner/repo/issues/1",
number: 1,
state: "open",
title: "Test Issue",
[change.field]: change.newValue,
},
},
});
service.watchTask("KB-001", "issue", "owner", "repo", 1);
await service.pollOnce();
expect(mockUpdateIssueInfo).toHaveBeenCalled();
}
});
});
describe("unwatch when badge removed", () => {
it("unwatches PR when task has no prInfo", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
// No prInfo
});
mockGetBadgeStatusesBatch.mockResolvedValue({
pr_1: null,
});
service.watchTask("KB-001", "pr", "owner", "repo", 1);
await service.pollOnce();
expect(service.getWatch("KB-001")?.pr).toBeUndefined();
});
it("unwatches issue when task has no issueInfo", async () => {
mockGetTask.mockResolvedValue({
id: "KB-001",
// No issueInfo
});
mockGetBadgeStatusesBatch.mockResolvedValue({
issue_1: null,
});
service.watchTask("KB-001", "issue", "owner", "repo", 1);
await service.pollOnce();
expect(service.getWatch("KB-001")?.issue).toBeUndefined();
});
});
});
describe("githubPoller singleton", () => {
it("is a singleton instance", () => {
expect(githubPoller).toBeInstanceOf(GitHubPollingService);
});
it("can be started and stopped", () => {
// Should not throw
githubPoller.start();
githubPoller.stop();
});
});
describe("githubRateLimiter singleton", () => {
it("is a singleton instance", () => {
expect(githubRateLimiter).toBeInstanceOf(GitHubRateLimiter);
});
});

View File

@@ -1,5 +1,5 @@
import { describe, it, expect, beforeEach } from "vitest";
import { rateLimit } from "./rate-limit.js";
import { describe, it, expect, beforeEach, vi, afterEach } from "vitest";
import { rateLimit, RATE_LIMITS } from "./rate-limit.js";
import type { Request, Response, NextFunction } from "express";
function mockReq(ip = "127.0.0.1"): Partial<Request> {
@@ -27,6 +27,20 @@ function mockRes(): Partial<Response> & { _status: number; _json: any; _headers:
return res;
}
describe("RATE_LIMITS constants", () => {
it("has correct values for api limit", () => {
expect(RATE_LIMITS.api).toEqual({ windowMs: 60_000, max: 100 });
});
it("has correct values for mutation limit", () => {
expect(RATE_LIMITS.mutation).toEqual({ windowMs: 60_000, max: 30 });
});
it("has correct values for sse limit", () => {
expect(RATE_LIMITS.sse).toEqual({ windowMs: 60_000, max: 10 });
});
});
describe("rateLimit", () => {
let middleware: ReturnType<typeof rateLimit>;
@@ -114,4 +128,109 @@ describe("rateLimit", () => {
mw(mockReq() as Request, res as unknown as Response, () => {});
expect(res._json).toEqual({ error: "Slow down!" });
});
it("uses default options (100 req/min, 60s window)", () => {
const defaultMiddleware = rateLimit();
const req = mockReq();
const res = mockRes();
let called = false;
// First request should be allowed
defaultMiddleware(req as Request, res as unknown as Response, () => { called = true; });
expect(called).toBe(true);
expect(res._headers["RateLimit-Limit"]).toBe("100");
});
it("respects custom options", () => {
const customMiddleware = rateLimit({ windowMs: 30_000, max: 5, message: "Custom" });
const req = mockReq();
const res = mockRes();
customMiddleware(req as Request, res as unknown as Response, () => {});
expect(res._headers["RateLimit-Limit"]).toBe("5");
});
it("uses remoteAddress when ip is undefined", () => {
const req = { socket: { remoteAddress: "192.168.1.1" } } as Partial<Request>;
const res = mockRes();
let called = false;
middleware(req as Request, res as unknown as Response, () => { called = true; });
expect(called).toBe(true);
});
describe("cleanup interval", () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it("removes expired entries after window expires", () => {
const shortWindowMs = 1000;
const cleanupMiddleware = rateLimit({ windowMs: shortWindowMs, max: 1 });
const req = mockReq("1.2.3.4");
// Make first request
cleanupMiddleware(req as Request, mockRes() as unknown as Response, () => {});
// Advance time past the window
vi.advanceTimersByTime(shortWindowMs + 100);
// New request should be allowed since old entry expired
const res = mockRes();
let called = false;
cleanupMiddleware(req as Request, res as unknown as Response, () => { called = true; });
expect(called).toBe(true);
expect(res._headers["RateLimit-Remaining"]).toBe("0");
});
});
});
describe("rateLimit with RATE_LIMITS presets", () => {
it("works with RATE_LIMITS.api preset", () => {
const apiMiddleware = rateLimit(RATE_LIMITS.api);
const req = mockReq();
const res = mockRes();
let called = false;
apiMiddleware(req as Request, res as unknown as Response, () => { called = true; });
expect(called).toBe(true);
expect(res._headers["RateLimit-Limit"]).toBe("100");
});
it("works with RATE_LIMITS.mutation preset", () => {
const mutationMiddleware = rateLimit(RATE_LIMITS.mutation);
const req = mockReq();
// Exhaust limit
for (let i = 0; i < 30; i++) {
mutationMiddleware(req as Request, mockRes() as unknown as Response, () => {});
}
// 31st request should be blocked
const res = mockRes();
mutationMiddleware(req as Request, res as unknown as Response, () => {});
expect(res._status).toBe(429);
});
it("works with RATE_LIMITS.sse preset", () => {
const sseMiddleware = rateLimit(RATE_LIMITS.sse);
const req = mockReq();
const res = mockRes();
// Exhaust limit
for (let i = 0; i < 10; i++) {
sseMiddleware(req as Request, mockRes() as unknown as Response, () => {});
}
// 11th request should be blocked
const blockedRes = mockRes();
sseMiddleware(req as Request, blockedRes as unknown as Response, () => {});
expect(blockedRes._status).toBe(429);
});
});