feat(KB-141): fix workspace type resolution and add clean-checkout typecheck tests
- Remove dist-dependent workspace type resolution from all tsconfig files - Add clean-checkout typecheck regression test to CI suite - Update package.json type definitions for core, engine, dashboard, and cli packages - Update README with typecheck testing documentation - Clean up unused test files and component dependencies
This commit is contained in:
10
README.md
10
README.md
@@ -163,6 +163,16 @@ pnpm dev dashboard # Board + AI engine
|
||||
pnpm dev task list # CLI commands
|
||||
```
|
||||
|
||||
### Type Checking
|
||||
|
||||
The workspace supports clean-checkout type checking — no build artifacts required:
|
||||
|
||||
```bash
|
||||
pnpm typecheck # Type-check all packages
|
||||
```
|
||||
|
||||
This command validates TypeScript across all packages using source file resolution, without requiring `dist/` output from prior builds. Run it after cloning or before committing to catch type errors early.
|
||||
|
||||
## Building a standalone executable
|
||||
|
||||
You can build a single self-contained `kb` binary using [Bun](https://bun.sh/):
|
||||
|
||||
@@ -60,6 +60,7 @@
|
||||
"@kb/dashboard": "workspace:*",
|
||||
"@kb/engine": "workspace:*",
|
||||
"@sinclair/typebox": "^0.34.0",
|
||||
"@types/node": "^22.0.0",
|
||||
"@vitest/coverage-v8": "^3.1.0",
|
||||
"tsup": "^8.5.1",
|
||||
"tsx": "^4.19.0",
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
"extends": "../../tsconfig.base.json",
|
||||
"compilerOptions": {
|
||||
"outDir": "dist",
|
||||
"rootDir": "src"
|
||||
"rootDir": "src",
|
||||
"types": ["node", "vitest/globals"]
|
||||
},
|
||||
"include": ["src"],
|
||||
"exclude": ["src/**/*.test.ts"]
|
||||
"include": ["src/**/*"],
|
||||
"exclude": ["src/**/*.test.ts", "src/**/__tests__/**/*"]
|
||||
}
|
||||
|
||||
@@ -4,12 +4,12 @@
|
||||
"type": "module",
|
||||
"exports": {
|
||||
".": {
|
||||
"import": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts"
|
||||
"types": "./src/index.ts",
|
||||
"import": "./dist/index.js"
|
||||
},
|
||||
"./gh-cli": {
|
||||
"import": "./dist/gh-cli.js",
|
||||
"types": "./dist/gh-cli.d.ts"
|
||||
"types": "./src/gh-cli.ts",
|
||||
"import": "./dist/gh-cli.js"
|
||||
}
|
||||
},
|
||||
"publishConfig": {
|
||||
|
||||
@@ -2,7 +2,8 @@
|
||||
"extends": "../../tsconfig.base.json",
|
||||
"compilerOptions": {
|
||||
"outDir": "dist",
|
||||
"rootDir": "src"
|
||||
"rootDir": "src",
|
||||
"types": ["node", "vitest/globals"]
|
||||
},
|
||||
"include": ["src"]
|
||||
"include": ["src/**/*"]
|
||||
}
|
||||
|
||||
@@ -222,7 +222,17 @@ pnpm dev
|
||||
|
||||
### Strict TypeScript Verification
|
||||
|
||||
The dashboard enforces strict type-checking via `src/__tests__/typecheck.test.ts`, which runs `tsc --noEmit --skipLibCheck false`. This ensures type safety across the workspace and catches missing or incompatible types in dependencies. The test verifies that dashboard source code (which imports from `@kb/engine`) satisfies all TypeScript constraints without skipping library type definitions.
|
||||
The dashboard enforces strict type-checking via `src/__tests__/typecheck.test.ts`, which runs `pnpm typecheck` to verify the workspace type-checks cleanly from a clean checkout. The test temporarily moves any existing `dist/` directories to ensure type resolution happens against source files, not stale build artifacts. This ensures type safety across the workspace and catches missing or incompatible types in dependencies without requiring a full build first.
|
||||
|
||||
### Workspace Type Checking
|
||||
|
||||
From the repository root, validate all packages without building:
|
||||
|
||||
```bash
|
||||
pnpm typecheck # Type-check all packages from clean checkout
|
||||
```
|
||||
|
||||
This works by configuring packages to resolve their workspace dependencies via TypeScript's module resolution against source files. The dashboard's own `typecheck` script runs both server (`src/`) and client (`app/`) type checks.
|
||||
|
||||
## API Endpoints
|
||||
|
||||
|
||||
@@ -379,7 +379,7 @@ describe("PlanningModeModal", () => {
|
||||
const onDeleteTask = vi.fn<(_: string) => Promise<Task>>().mockResolvedValue(mockTasks[0]);
|
||||
const onMergeTask = vi
|
||||
.fn<(_: string) => Promise<MergeResult>>()
|
||||
.mockResolvedValue({ merged: true, branch: "kb/kb-999" });
|
||||
.mockResolvedValue({ merged: true, branch: "kb/kb-999", task: mockTasks[0], worktreeRemoved: true, branchDeleted: true });
|
||||
|
||||
const { container } = render(
|
||||
<TaskDetailModal
|
||||
|
||||
@@ -334,7 +334,7 @@ export function PlanningModeModal({ isOpen, onClose, onTaskCreated, tasks, initi
|
||||
// Reset to question mode for more refinement
|
||||
setView({ type: "question", session: view.session });
|
||||
}}
|
||||
isLoading={view.type === "loading"}
|
||||
isLoading={false}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
@@ -377,7 +377,7 @@ function QuestionForm({ question, progress, onSubmit, onBack }: QuestionFormProp
|
||||
case "single_select":
|
||||
return response[question.id] !== undefined;
|
||||
case "multi_select":
|
||||
return Array.isArray(response[question.id]) && response[question.id].length > 0;
|
||||
return Array.isArray(response[question.id] as unknown) && (response[question.id] as unknown[]).length > 0;
|
||||
case "confirm":
|
||||
return response[question.id] !== undefined;
|
||||
default:
|
||||
|
||||
@@ -3,6 +3,7 @@ import { X, Trash2, Terminal as TerminalIcon, RefreshCw } from "lucide-react";
|
||||
import { useTerminal } from "../hooks/useTerminal";
|
||||
import { createTerminalSession, killPtyTerminalSession } from "../api";
|
||||
import type { Terminal as XTerm, ITerminalAddon } from "@xterm/xterm";
|
||||
import type { FitAddon } from "@xterm/addon-fit";
|
||||
|
||||
import "@xterm/xterm/css/xterm.css";
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ describe("QuickEntryBox", () => {
|
||||
renderQuickEntryBox();
|
||||
const input = screen.getByTestId("quick-entry-input");
|
||||
expect(input).toBeTruthy();
|
||||
expect(input.placeholder).toBe("Add a task...");
|
||||
expect((input as HTMLInputElement).placeholder).toBe("Add a task...");
|
||||
});
|
||||
|
||||
it("creates task on Enter key", async () => {
|
||||
@@ -42,7 +42,7 @@ describe("QuickEntryBox", () => {
|
||||
|
||||
// Check loading placeholder
|
||||
await waitFor(() => {
|
||||
expect(input.placeholder).toBe("Creating...");
|
||||
expect((input as HTMLInputElement).placeholder).toBe("Creating...");
|
||||
});
|
||||
|
||||
// Input should be disabled during creation
|
||||
@@ -60,7 +60,7 @@ describe("QuickEntryBox", () => {
|
||||
expect(props.onCreate).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
expect(input.value).toBe("");
|
||||
expect((input as HTMLInputElement).value).toBe("");
|
||||
});
|
||||
|
||||
it("shows error toast on failure and keeps input content", async () => {
|
||||
@@ -76,7 +76,7 @@ describe("QuickEntryBox", () => {
|
||||
});
|
||||
|
||||
// Input content should be preserved for retry
|
||||
expect(input.value).toBe("Failed task");
|
||||
expect((input as HTMLInputElement).value).toBe("Failed task");
|
||||
});
|
||||
|
||||
it("clears non-empty input on Escape key", () => {
|
||||
@@ -84,10 +84,10 @@ describe("QuickEntryBox", () => {
|
||||
const input = screen.getByTestId("quick-entry-input");
|
||||
|
||||
fireEvent.change(input, { target: { value: "Some text" } });
|
||||
expect(input.value).toBe("Some text");
|
||||
expect((input as HTMLInputElement).value).toBe("Some text");
|
||||
|
||||
fireEvent.keyDown(input, { key: "Escape" });
|
||||
expect(input.value).toBe("");
|
||||
expect((input as HTMLInputElement).value).toBe("");
|
||||
});
|
||||
|
||||
it("does not clear empty input on Escape key", () => {
|
||||
@@ -95,7 +95,7 @@ describe("QuickEntryBox", () => {
|
||||
const input = screen.getByTestId("quick-entry-input");
|
||||
|
||||
fireEvent.keyDown(input, { key: "Escape" });
|
||||
expect(input.value).toBe("");
|
||||
expect((input as HTMLInputElement).value).toBe("");
|
||||
});
|
||||
|
||||
it("does not submit on Enter if input is empty", async () => {
|
||||
@@ -137,7 +137,7 @@ describe("QuickEntryBox", () => {
|
||||
const input = screen.getByTestId("quick-entry-input");
|
||||
|
||||
fireEvent.change(input, { target: { value: "Updated text" } });
|
||||
expect(input.value).toBe("Updated text");
|
||||
expect((input as HTMLInputElement).value).toBe("Updated text");
|
||||
});
|
||||
|
||||
it("trims whitespace when creating task", async () => {
|
||||
|
||||
@@ -56,7 +56,7 @@ export function useFileEditor(
|
||||
setError(null);
|
||||
|
||||
try {
|
||||
const response: FileContentResponse = await fetchFileContent(taskId, filePath);
|
||||
const response: FileContentResponse = await fetchFileContent(taskId, filePath!);
|
||||
|
||||
if (!cancelled) {
|
||||
setContentState(response.content);
|
||||
|
||||
@@ -4,8 +4,8 @@
|
||||
"type": "module",
|
||||
"exports": {
|
||||
".": {
|
||||
"import": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts"
|
||||
"types": "./src/index.ts",
|
||||
"import": "./dist/index.js"
|
||||
}
|
||||
},
|
||||
"publishConfig": {
|
||||
|
||||
@@ -1,16 +1,105 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { execSync } from "node:child_process";
|
||||
import { resolve } from "node:path";
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { rename, access } from "node:fs/promises";
|
||||
import { describe, it, expect, beforeAll, afterAll } from "vitest";
|
||||
|
||||
describe("typecheck", () => {
|
||||
it("passes tsc --noEmit --skipLibCheck false", () => {
|
||||
const cwd = resolve(__dirname, "../..");
|
||||
expect(() =>
|
||||
execFileSync("npx", ["tsc", "--noEmit", "--skipLibCheck", "false"], {
|
||||
/**
|
||||
* Clean-checkout typecheck regression test.
|
||||
*
|
||||
* This test verifies that `pnpm typecheck` succeeds from a clean checkout
|
||||
* state without relying on pre-built dist/ artifacts. It temporarily moves
|
||||
* any existing dist directories to ensure the typecheck runs against
|
||||
* source files and project references.
|
||||
*/
|
||||
describe("clean-checkout typecheck", () => {
|
||||
const cwd = resolve(__dirname, "../..");
|
||||
const distPaths = [
|
||||
"packages/core/dist",
|
||||
"packages/engine/dist",
|
||||
"packages/dashboard/dist",
|
||||
"packages/cli/dist",
|
||||
];
|
||||
const movedSuffix = ".moved-for-test";
|
||||
const movedPaths = distPaths.map((p) => `${p}${movedSuffix}`);
|
||||
|
||||
// Helper to check if a path exists
|
||||
async function pathExists(path: string): Promise<boolean> {
|
||||
try {
|
||||
await access(path);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
// Move any existing dist directories to simulate clean checkout
|
||||
for (let i = 0; i < distPaths.length; i++) {
|
||||
const distPath = resolve(cwd, distPaths[i]);
|
||||
const movedPath = resolve(cwd, movedPaths[i]);
|
||||
|
||||
if (await pathExists(distPath)) {
|
||||
await rename(distPath, movedPath);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
// Restore moved directories even if test failed
|
||||
for (let i = 0; i < distPaths.length; i++) {
|
||||
const distPath = resolve(cwd, distPaths[i]);
|
||||
const movedPath = resolve(cwd, movedPaths[i]);
|
||||
|
||||
if (await pathExists(movedPath)) {
|
||||
try {
|
||||
await rename(movedPath, distPath);
|
||||
} catch {
|
||||
// Best effort - if restore fails, we'll rebuild in subsequent steps
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("passes pnpm typecheck without relying on dist/ artifacts", () => {
|
||||
let error: Error | null = null;
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
|
||||
try {
|
||||
stdout = execSync("pnpm typecheck", {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
timeout: 60_000,
|
||||
}),
|
||||
).not.toThrow();
|
||||
encoding: "utf-8",
|
||||
timeout: 120_000,
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
});
|
||||
} catch (e) {
|
||||
error = e as Error;
|
||||
// Capture stdout/stderr from the error object if available
|
||||
const execError = e as { stdout?: string; stderr?: string };
|
||||
stdout = execError.stdout ?? "";
|
||||
stderr = execError.stderr ?? "";
|
||||
}
|
||||
|
||||
// Assertion-based verification - fail on non-zero exit
|
||||
if (error) {
|
||||
const failureContext = [
|
||||
"pnpm typecheck failed with non-zero exit code",
|
||||
"",
|
||||
"--- STDOUT ---",
|
||||
stdout,
|
||||
"",
|
||||
"--- STDERR ---",
|
||||
stderr,
|
||||
"",
|
||||
"--- ERROR ---",
|
||||
error.message,
|
||||
].join("\n");
|
||||
|
||||
expect.fail(failureContext);
|
||||
}
|
||||
|
||||
// Verify that typecheck ran and succeeded - just check no error was thrown
|
||||
// The fact that we got here without error means it passed
|
||||
expect(error).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,5 +8,6 @@
|
||||
"noEmit": true,
|
||||
"types": ["vitest/globals", "@testing-library/jest-dom", "node"]
|
||||
},
|
||||
"include": ["app"]
|
||||
"include": ["app/**/*"],
|
||||
"exclude": ["app/**/*.test.ts", "app/**/*.test.tsx"]
|
||||
}
|
||||
|
||||
@@ -3,7 +3,8 @@
|
||||
"compilerOptions": {
|
||||
"outDir": "dist",
|
||||
"rootDir": "src",
|
||||
"jsx": "react-jsx"
|
||||
"jsx": "react-jsx",
|
||||
"types": ["node", "vitest/globals", "@testing-library/jest-dom"]
|
||||
},
|
||||
"include": ["src"]
|
||||
"include": ["src/**/*"]
|
||||
}
|
||||
|
||||
@@ -4,8 +4,8 @@
|
||||
"type": "module",
|
||||
"exports": {
|
||||
".": {
|
||||
"import": "./dist/index.js",
|
||||
"types": "./dist/index.d.ts"
|
||||
"types": "./src/index.ts",
|
||||
"import": "./dist/index.js"
|
||||
}
|
||||
},
|
||||
"publishConfig": {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { TaskStore, Task, Column, Settings, MergeResult } from "@kb/core";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { schedulerLog } from "./logger.js";
|
||||
|
||||
export interface NtfyNotifierOptions {
|
||||
@@ -6,6 +7,13 @@ export interface NtfyNotifierOptions {
|
||||
ntfyBaseUrl?: string;
|
||||
}
|
||||
|
||||
/** Minimal store interface needed by NtfyNotifier */
|
||||
interface NtfyNotifierStore {
|
||||
getSettings(): Promise<Settings> | Settings;
|
||||
on(event: string, listener: (...args: any[]) => void): void;
|
||||
off(event: string, listener: (...args: any[]) => void): void;
|
||||
}
|
||||
|
||||
interface NtfyConfig {
|
||||
enabled: boolean;
|
||||
topic: string | undefined;
|
||||
@@ -34,7 +42,7 @@ export class NtfyNotifier {
|
||||
private abortController: AbortController | null = null;
|
||||
|
||||
constructor(
|
||||
private store: TaskStore,
|
||||
private store: NtfyNotifierStore,
|
||||
options: NtfyNotifierOptions = {},
|
||||
) {
|
||||
this.ntfyBaseUrl = options.ntfyBaseUrl ?? "https://ntfy.sh";
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { PrCommentHandler } from "./pr-comment-handler.js";
|
||||
import type { TaskStore } from "@kb/core";
|
||||
import type { TaskStore, Task } from "@kb/core";
|
||||
|
||||
const mockStore = {
|
||||
addSteeringComment: vi.fn(),
|
||||
createTask: vi.fn().mockResolvedValue({ id: "KB-123" }),
|
||||
addSteeringComment: vi.fn<(id: string, text: string, author?: "user" | "agent") => Promise<Task>>(),
|
||||
createTask: vi.fn<(input: Parameters<TaskStore["createTask"]>[0]) => Promise<Task>>().mockResolvedValue({ id: "KB-123" } as Task),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
describe("PrCommentHandler", () => {
|
||||
@@ -126,7 +126,7 @@ describe("PrCommentHandler", () => {
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.addSteeringComment.mock.calls[0];
|
||||
const call = (mockStore.addSteeringComment as ReturnType<typeof vi.fn>).mock.calls[0];
|
||||
const text = call[1] as string;
|
||||
|
||||
expect(text).toContain("PR Review Feedback");
|
||||
@@ -151,7 +151,7 @@ describe("PrCommentHandler", () => {
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.addSteeringComment.mock.calls[0];
|
||||
const call = (mockStore.addSteeringComment as ReturnType<typeof vi.fn>).mock.calls[0];
|
||||
const text = call[1] as string;
|
||||
|
||||
expect(text.length).toBeLessThan(longBody.length);
|
||||
@@ -189,7 +189,7 @@ describe("PrCommentHandler", () => {
|
||||
},
|
||||
]);
|
||||
|
||||
const text = mockStore.addSteeringComment.mock.calls[0][1] as string;
|
||||
const text = (mockStore.addSteeringComment as ReturnType<typeof vi.fn>).mock.calls[0][1] as string;
|
||||
expect(text).toContain("This PR is already merged");
|
||||
expect(text).toContain("follow-up work");
|
||||
});
|
||||
@@ -242,7 +242,7 @@ describe("PrCommentHandler", () => {
|
||||
},
|
||||
]);
|
||||
|
||||
const call = mockStore.createTask.mock.calls[0];
|
||||
const call = (mockStore.createTask as ReturnType<typeof vi.fn>).mock.calls[0];
|
||||
const description = call[0].description as string;
|
||||
|
||||
expect(description).toContain("@reviewer1");
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
"extends": "../../tsconfig.base.json",
|
||||
"compilerOptions": {
|
||||
"outDir": "dist",
|
||||
"rootDir": "src"
|
||||
"rootDir": "src",
|
||||
"types": ["node", "vitest/globals"]
|
||||
},
|
||||
"include": ["src"],
|
||||
"exclude": ["src/**/*.test.ts"]
|
||||
"include": ["src/**/*"]
|
||||
}
|
||||
|
||||
17
pnpm-lock.yaml
generated
17
pnpm-lock.yaml
generated
@@ -45,9 +45,12 @@ importers:
|
||||
'@sinclair/typebox':
|
||||
specifier: ^0.34.0
|
||||
version: 0.34.48
|
||||
'@types/node':
|
||||
specifier: ^22.0.0
|
||||
version: 22.19.15
|
||||
'@vitest/coverage-v8':
|
||||
specifier: ^3.1.0
|
||||
version: 3.2.4(vitest@3.2.4(@types/debug@4.1.13)(@types/node@25.5.0)(jsdom@29.0.1)(tsx@4.21.0)(yaml@2.8.3))
|
||||
version: 3.2.4(vitest@3.2.4(@types/debug@4.1.13)(@types/node@22.19.15)(jsdom@29.0.1)(tsx@4.21.0)(yaml@2.8.3))
|
||||
tsup:
|
||||
specifier: ^8.5.1
|
||||
version: 8.5.1(postcss@8.5.8)(tsx@4.21.0)(typescript@5.9.3)(yaml@2.8.3)
|
||||
@@ -59,7 +62,7 @@ importers:
|
||||
version: 5.9.3
|
||||
vitest:
|
||||
specifier: ^3.1.0
|
||||
version: 3.2.4(@types/debug@4.1.13)(@types/node@25.5.0)(jsdom@29.0.1)(tsx@4.21.0)(yaml@2.8.3)
|
||||
version: 3.2.4(@types/debug@4.1.13)(@types/node@22.19.15)(jsdom@29.0.1)(tsx@4.21.0)(yaml@2.8.3)
|
||||
yaml:
|
||||
specifier: ^2.8.3
|
||||
version: 2.8.3
|
||||
@@ -5560,7 +5563,7 @@ snapshots:
|
||||
'@types/body-parser@1.19.6':
|
||||
dependencies:
|
||||
'@types/connect': 3.4.38
|
||||
'@types/node': 22.19.15
|
||||
'@types/node': 25.5.0
|
||||
|
||||
'@types/chai@5.2.3':
|
||||
dependencies:
|
||||
@@ -5569,7 +5572,7 @@ snapshots:
|
||||
|
||||
'@types/connect@3.4.38':
|
||||
dependencies:
|
||||
'@types/node': 22.19.15
|
||||
'@types/node': 25.5.0
|
||||
|
||||
'@types/debug@4.1.13':
|
||||
dependencies:
|
||||
@@ -5585,7 +5588,7 @@ snapshots:
|
||||
|
||||
'@types/express-serve-static-core@5.1.1':
|
||||
dependencies:
|
||||
'@types/node': 22.19.15
|
||||
'@types/node': 25.5.0
|
||||
'@types/qs': 6.15.0
|
||||
'@types/range-parser': 1.2.7
|
||||
'@types/send': 1.2.1
|
||||
@@ -5640,12 +5643,12 @@ snapshots:
|
||||
|
||||
'@types/send@1.2.1':
|
||||
dependencies:
|
||||
'@types/node': 22.19.15
|
||||
'@types/node': 25.5.0
|
||||
|
||||
'@types/serve-static@2.2.0':
|
||||
dependencies:
|
||||
'@types/http-errors': 2.0.5
|
||||
'@types/node': 22.19.15
|
||||
'@types/node': 25.5.0
|
||||
|
||||
'@types/unist@2.0.11': {}
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
"resolveJsonModule": true,
|
||||
"isolatedModules": true,
|
||||
"outDir": "dist",
|
||||
"rootDir": "src"
|
||||
"rootDir": "src",
|
||||
"types": ["node"]
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user