ci: add typechecking, fail fast in CI, and patch typechecking errors (#63)

This commit is contained in:
Charles Packer
2025-11-04 11:50:07 -08:00
committed by GitHub
parent 42eb671bf4
commit cf73f3a11f
27 changed files with 183 additions and 69 deletions

View File

@@ -69,10 +69,12 @@ export async function getResumeData(
const approvalMessage = matchingMessages.find(
(msg) => msg.message_type === "approval_request_message",
);
const inContextMessage =
approvalMessage ?? matchingMessages[matchingMessages.length - 1]!;
const lastMessage = matchingMessages[matchingMessages.length - 1];
const inContextMessage = approvalMessage ?? lastMessage;
messageToCheck = inContextMessage;
if (inContextMessage) {
messageToCheck = inContextMessage;
}
} else {
console.warn(
`[check-approval] In-context message ${inContextLastMessageId} not found in cursor fetch.\n` +

View File

@@ -25,7 +25,13 @@ export function resolveModel(modelIdentifier: string): string | null {
*/
export function getDefaultModel(): string {
const defaultModel = models.find((m) => m.isDefault);
return defaultModel?.handle || models[0].handle;
if (defaultModel) return defaultModel.handle;
const firstModel = models[0];
if (!firstModel) {
throw new Error("No models available in models.json");
}
return firstModel.handle;
}
/**

View File

@@ -281,7 +281,9 @@ export function AdvancedDiffRenderer(
let newNo = h.newStart;
let lastRemovalNo: number | null = null;
for (let i = 0; i < h.lines.length; i++) {
const raw = h.lines[i].raw || "";
const line = h.lines[i];
if (!line) continue;
const raw = line.raw || "";
const ch = raw.charAt(0);
const body = raw.slice(1);
// Skip meta lines (e.g., "\ No newline at end of file"): do not display, do not advance counters,
@@ -291,7 +293,9 @@ export function AdvancedDiffRenderer(
// Helper to find next non-meta '+' index
const findNextPlus = (start: number): string | undefined => {
for (let j = start + 1; j < h.lines.length; j++) {
const r = h.lines[j].raw || "";
const nextLine = h.lines[j];
if (!nextLine) continue;
const r = nextLine.raw || "";
if (r.charAt(0) === "\\") continue; // skip meta
if (r.startsWith("+")) return r.slice(1);
break; // stop at first non-meta non-plus
@@ -301,7 +305,9 @@ export function AdvancedDiffRenderer(
// Helper to find previous non-meta '-' index
const findPrevMinus = (start: number): string | undefined => {
for (let k = start - 1; k >= 0; k--) {
const r = h.lines[k].raw || "";
const prevLine = h.lines[k];
if (!prevLine) continue;
const r = prevLine.raw || "";
if (r.charAt(0) === "\\") continue; // skip meta
if (r.startsWith("-")) return r.slice(1);
break; // stop at first non-meta non-minus

View File

@@ -77,7 +77,7 @@ export const MarkdownDisplay: React.FC<MarkdownDisplayProps> = ({
// Check for headers
const headerMatch = line.match(headerRegex);
if (headerMatch) {
if (headerMatch?.[1] && headerMatch[2] !== undefined) {
const level = headerMatch[1].length;
const content = headerMatch[2];
@@ -119,7 +119,12 @@ export const MarkdownDisplay: React.FC<MarkdownDisplayProps> = ({
// Check for list items
const listMatch = line.match(listItemRegex);
if (listMatch) {
if (
listMatch &&
listMatch[1] !== undefined &&
listMatch[2] &&
listMatch[3] !== undefined
) {
const indent = listMatch[1].length;
const marker = listMatch[2];
const content = listMatch[3];
@@ -146,7 +151,7 @@ export const MarkdownDisplay: React.FC<MarkdownDisplayProps> = ({
// Check for blockquotes
const blockquoteMatch = line.match(blockquoteRegex);
if (blockquoteMatch) {
if (blockquoteMatch && blockquoteMatch[1] !== undefined) {
contentBlocks.push(
<Box key={key} paddingLeft={2}>
<Text dimColor> </Text>

View File

@@ -60,6 +60,8 @@ export function PasteAwareTextInput({
onSubmit?: (value: string) => void;
placeholder?: string;
focus?: boolean;
externalCursorOffset?: number;
onCursorOffsetChange?: (n: number) => void;
}>;
// Sync external value changes (treat incoming value as DISPLAY value)

View File

@@ -123,6 +123,9 @@ export function backfillBuffers(
if (toolCalls.length > 0 && toolCalls[0]?.tool_call_id) {
const toolCall = toolCalls[0];
const toolCallId = toolCall.tool_call_id;
// Skip if any required fields are missing
if (!toolCallId || !toolCall.name || !toolCall.arguments) break;
const exists = buffers.byId.has(lineId);
buffers.byId.set(lineId, {

View File

@@ -23,11 +23,13 @@ export function formatArgsDisplay(argsJson: string): {
if ("request_heartbeat" in clone) delete clone.request_heartbeat;
parsed = clone;
const keys = Object.keys(parsed);
const firstKey = keys[0];
if (
keys.length === 1 &&
["query", "path", "file_path", "command", "label"].includes(keys[0])
firstKey &&
["query", "path", "file_path", "command", "label"].includes(firstKey)
) {
const v = parsed[keys[0]];
const v = parsed[firstKey];
display = typeof v === "string" ? v : String(v);
} else {
display = Object.entries(parsed)

View File

@@ -191,7 +191,7 @@ export async function drainStreamWithResume(
// Use the resume result (should have proper stop_reason now)
result = resumeResult;
} catch (e) {
} catch (_e) {
// Resume failed - stick with the error stop_reason
// The original error result will be returned
}

View File

@@ -161,7 +161,7 @@ function analyzeBashApproval(
_workingDir: string,
): ApprovalContext {
const parts = command.trim().split(/\s+/);
const baseCommand = parts[0];
const baseCommand = parts[0] || "";
const firstArg = parts[1] || "";
// Dangerous commands - no persistence
@@ -178,7 +178,7 @@ function analyzeBashApproval(
"killall",
];
if (dangerousCommands.includes(baseCommand)) {
if (baseCommand && dangerousCommands.includes(baseCommand)) {
return {
recommendedRule: "",
ruleDescription: "",
@@ -248,7 +248,7 @@ function analyzeBashApproval(
}
// Package manager commands
if (["npm", "bun", "yarn", "pnpm"].includes(baseCommand)) {
if (baseCommand && ["npm", "bun", "yarn", "pnpm"].includes(baseCommand)) {
const subcommand = firstArg;
const thirdPart = parts[2];
@@ -295,7 +295,7 @@ function analyzeBashApproval(
"tail",
];
if (safeCommands.includes(baseCommand)) {
if (baseCommand && safeCommands.includes(baseCommand)) {
return {
recommendedRule: `Bash(${baseCommand}:*)`,
ruleDescription: `'${baseCommand}' commands`,
@@ -318,7 +318,7 @@ function analyzeBashApproval(
for (const segment of segments) {
const segmentParts = segment.trim().split(/\s+/);
const segmentBase = segmentParts[0];
const segmentBase = segmentParts[0] || "";
const segmentArg = segmentParts[1] || "";
// Check if this segment is git command
@@ -350,7 +350,7 @@ function analyzeBashApproval(
}
// Check if this segment is npm/bun/yarn/pnpm
if (["npm", "bun", "yarn", "pnpm"].includes(segmentBase)) {
if (segmentBase && ["npm", "bun", "yarn", "pnpm"].includes(segmentBase)) {
const subcommand = segmentArg;
const thirdPart = segmentParts[2];

View File

@@ -26,7 +26,7 @@ export function matchesFilePattern(
// Extract tool name and file path from query
// Format: "ToolName(filePath)"
const queryMatch = query.match(/^([^(]+)\((.+)\)$/);
if (!queryMatch) {
if (!queryMatch || !queryMatch[1] || !queryMatch[2]) {
return false;
}
const queryTool = queryMatch[1];
@@ -35,7 +35,7 @@ export function matchesFilePattern(
// Extract tool name and glob pattern from permission rule
// Format: "ToolName(pattern)"
const patternMatch = pattern.match(/^([^(]+)\((.+)\)$/);
if (!patternMatch) {
if (!patternMatch || !patternMatch[1] || !patternMatch[2]) {
return false;
}
const patternTool = patternMatch[1];
@@ -98,7 +98,7 @@ export function matchesBashPattern(query: string, pattern: string): boolean {
// Extract the command from query
// Format: "Bash(actual command)" or "Bash()"
const queryMatch = query.match(/^Bash\((.*)\)$/);
if (!queryMatch) {
if (!queryMatch || queryMatch[1] === undefined) {
return false;
}
const command = queryMatch[1];
@@ -106,7 +106,7 @@ export function matchesBashPattern(query: string, pattern: string): boolean {
// Extract the command pattern from permission rule
// Format: "Bash(command pattern)" or "Bash()"
const patternMatch = pattern.match(/^Bash\((.*)\)$/);
if (!patternMatch) {
if (!patternMatch || patternMatch[1] === undefined) {
return false;
}
const commandPattern = patternMatch[1];

View File

@@ -76,7 +76,7 @@ test("buildMessageContentFromDisplay handles mixed content", () => {
type: "text",
text: "Start Pasted content middle ",
});
expect(content[1].type).toBe("image");
expect(content[1]?.type).toBe("image");
expect(content[2]).toEqual({ type: "text", text: " end" });
});

View File

@@ -11,8 +11,8 @@ describe("Bash background tools", () => {
run_in_background: true,
});
expect(result.content[0].text).toContain("background with ID:");
expect(result.content[0].text).toMatch(/bash_\d+/);
expect(result.content[0]?.text).toContain("background with ID:");
expect(result.content[0]?.text).toMatch(/bash_\d+/);
});
test("BashOutput retrieves output from background shell", async () => {
@@ -24,7 +24,7 @@ describe("Bash background tools", () => {
});
// Extract bash_id from the response text
const match = startResult.content[0].text.match(/bash_(\d+)/);
const match = startResult.content[0]?.text.match(/bash_(\d+)/);
expect(match).toBeDefined();
const bashId = `bash_${match?.[1]}`;
@@ -51,7 +51,7 @@ describe("Bash background tools", () => {
run_in_background: true,
});
const match = startResult.content[0].text.match(/bash_(\d+)/);
const match = startResult.content[0]?.text.match(/bash_(\d+)/);
const bashId = `bash_${match?.[1]}`;
// Kill it (KillBash uses shell_id parameter)

View File

@@ -9,7 +9,7 @@ describe("Bash tool", () => {
});
expect(result.content).toBeDefined();
expect(result.content[0].text).toContain("Hello, World!");
expect(result.content[0]?.text).toContain("Hello, World!");
expect(result.isError).toBeUndefined();
});
@@ -19,7 +19,7 @@ describe("Bash tool", () => {
description: "Test stderr",
});
expect(result.content[0].text).toContain("error message");
expect(result.content[0]?.text).toContain("error message");
});
test("returns error for failed command", async () => {
@@ -29,7 +29,7 @@ describe("Bash tool", () => {
});
expect(result.isError).toBe(true);
expect(result.content[0].text).toContain("Exit code");
expect(result.content[0]?.text).toContain("Exit code");
});
test("times out long-running command", async () => {
@@ -40,7 +40,7 @@ describe("Bash tool", () => {
});
expect(result.isError).toBe(true);
expect(result.content[0].text).toContain("timed out");
expect(result.content[0]?.text).toContain("timed out");
}, 2000);
test("runs command in background mode", async () => {
@@ -50,8 +50,8 @@ describe("Bash tool", () => {
run_in_background: true,
});
expect(result.content[0].text).toContain("background with ID:");
expect(result.content[0].text).toMatch(/bash_\d+/);
expect(result.content[0]?.text).toContain("background with ID:");
expect(result.content[0]?.text).toMatch(/bash_\d+/);
});
test("handles complex commands with pipes", async () => {
@@ -65,8 +65,8 @@ describe("Bash tool", () => {
description: "Test pipe",
});
expect(result.content[0].text).toContain("bar");
expect(result.content[0].text).not.toContain("foo");
expect(result.content[0]?.text).toContain("bar");
expect(result.content[0]?.text).not.toContain("foo");
});
test("lists background processes with /bashes command", async () => {
@@ -76,7 +76,7 @@ describe("Bash tool", () => {
});
expect(result.content).toBeDefined();
expect(result.content[0].text).toBeDefined();
expect(result.content[0]?.text).toBeDefined();
});
test("throws error when command is missing", async () => {

View File

@@ -108,7 +108,7 @@ describe("Edit tool", () => {
file_path: file,
old_string: "World",
new_str: "Bun",
} as Parameters<typeof edit>[0]),
} as unknown as Parameters<typeof edit>[0]),
).rejects.toThrow(/missing required parameter.*new_string/);
});
});

View File

@@ -17,9 +17,9 @@ describe("LS tool", () => {
const result = await ls({ path: testDir.path });
expect(result.content[0].text).toContain("file1.txt");
expect(result.content[0].text).toContain("file2.txt");
expect(result.content[0].text).toContain("subdir/");
expect(result.content[0]?.text).toContain("file1.txt");
expect(result.content[0]?.text).toContain("file2.txt");
expect(result.content[0]?.text).toContain("subdir/");
});
test("shows directories with trailing slash", async () => {
@@ -29,8 +29,8 @@ describe("LS tool", () => {
const result = await ls({ path: testDir.path });
expect(result.content[0].text).toContain("folder/");
expect(result.content[0].text).toContain("file.txt");
expect(result.content[0]?.text).toContain("folder/");
expect(result.content[0]?.text).toContain("file.txt");
});
test("throws error for non-existent directory", async () => {
@@ -64,10 +64,10 @@ describe("LS tool", () => {
ignore: ["*.log", "node_modules"],
});
expect(result.content[0].text).toContain("file1.txt");
expect(result.content[0].text).toContain("important.txt");
expect(result.content[0].text).not.toContain("file2.log");
expect(result.content[0].text).not.toContain("node_modules");
expect(result.content[0]?.text).toContain("file1.txt");
expect(result.content[0]?.text).toContain("important.txt");
expect(result.content[0]?.text).not.toContain("file2.log");
expect(result.content[0]?.text).not.toContain("node_modules");
});
test("throws error when ignore is a string instead of array", async () => {

View File

@@ -96,7 +96,7 @@ describe("MultiEdit tool", () => {
multi_edit({
file_path: file,
edits: [
{ old_string: "foo", new_str: "baz" } as Parameters<
{ old_string: "foo", new_str: "baz" } as unknown as Parameters<
typeof multi_edit
>[0]["edits"][0],
],

View File

@@ -267,6 +267,7 @@ describe("tool truncation integration tests", () => {
const bashIdMatch = message.match(/with ID: (.+)/);
expect(bashIdMatch).toBeTruthy();
const bashId = bashIdMatch?.[1];
if (!bashId) throw new Error("bashId not found");
// Wait a bit for output to accumulate
await new Promise((resolve) => setTimeout(resolve, 100));

View File

@@ -113,7 +113,9 @@ export async function grep(args: GrepArgs): Promise<GrepResult> {
for (const line of lines) {
const parts = line.split(":");
if (parts.length >= 2) {
const count = parseInt(parts[parts.length - 1], 10);
const lastPart = parts[parts.length - 1];
if (!lastPart) continue;
const count = parseInt(lastPart, 10);
if (!Number.isNaN(count) && count > 0) {
totalMatches += count;
filesWithMatches++;

View File

@@ -19,7 +19,11 @@ export async function ls(
args: LSArgs,
): Promise<{ content: Array<{ type: string; text: string }> }> {
validateRequiredParams(args, ["path"], "LS");
validateParamTypes(args, LSSchema, "LS");
validateParamTypes(
args as unknown as Record<string, unknown>,
LSSchema,
"LS",
);
const { path: inputPath, ignore = [] } = args;
const dirPath = resolve(inputPath);
try {

View File

@@ -25,12 +25,16 @@ export async function multi_edit(
throw new Error(`File path must be absolute, got: ${file_path}`);
if (!edits || edits.length === 0) throw new Error("No edits provided");
for (let i = 0; i < edits.length; i++) {
const edit = edits[i];
if (!edit) {
throw new Error(`Edit ${i + 1} is undefined`);
}
validateRequiredParams(
edits[i] as Record<string, unknown>,
edit as unknown as Record<string, unknown>,
["old_string", "new_string"],
`MultiEdit (edit ${i + 1})`,
);
if (edits[i].old_string === edits[i].new_string)
if (edit.old_string === edit.new_string)
throw new Error(
`Edit ${i + 1}: No changes to make: old_string and new_string are exactly the same.`,
);
@@ -39,7 +43,9 @@ export async function multi_edit(
let content = await fs.readFile(file_path, "utf-8");
const appliedEdits: string[] = [];
for (let i = 0; i < edits.length; i++) {
const { old_string, new_string, replace_all = false } = edits[i];
const edit = edits[i];
if (!edit) continue;
const { old_string, new_string, replace_all = false } = edit;
const occurrences = content.split(old_string).length - 1;
if (occurrences === 0) {
throw new Error(

View File

@@ -35,7 +35,7 @@ import ReadSchema from "./schemas/Read.json";
import TodoWriteSchema from "./schemas/TodoWrite.json";
import WriteSchema from "./schemas/Write.json";
type ToolImplementation = (args: Record<string, any>) => Promise<any>;
type ToolImplementation = (args: Record<string, unknown>) => Promise<unknown>;
interface ToolAssets {
schema: Record<string, unknown>;
@@ -47,62 +47,62 @@ const toolDefinitions = {
Bash: {
schema: BashSchema,
description: BashDescription.trim(),
impl: bash as ToolImplementation,
impl: bash as unknown as ToolImplementation,
},
BashOutput: {
schema: BashOutputSchema,
description: BashOutputDescription.trim(),
impl: bash_output as ToolImplementation,
impl: bash_output as unknown as ToolImplementation,
},
Edit: {
schema: EditSchema,
description: EditDescription.trim(),
impl: edit as ToolImplementation,
impl: edit as unknown as ToolImplementation,
},
ExitPlanMode: {
schema: ExitPlanModeSchema,
description: ExitPlanModeDescription.trim(),
impl: exit_plan_mode as ToolImplementation,
impl: exit_plan_mode as unknown as ToolImplementation,
},
Glob: {
schema: GlobSchema,
description: GlobDescription.trim(),
impl: glob as ToolImplementation,
impl: glob as unknown as ToolImplementation,
},
Grep: {
schema: GrepSchema,
description: GrepDescription.trim(),
impl: grep as ToolImplementation,
impl: grep as unknown as ToolImplementation,
},
KillBash: {
schema: KillBashSchema,
description: KillBashDescription.trim(),
impl: kill_bash as ToolImplementation,
impl: kill_bash as unknown as ToolImplementation,
},
LS: {
schema: LSSchema,
description: LSDescription.trim(),
impl: ls as ToolImplementation,
impl: ls as unknown as ToolImplementation,
},
MultiEdit: {
schema: MultiEditSchema,
description: MultiEditDescription.trim(),
impl: multi_edit as ToolImplementation,
impl: multi_edit as unknown as ToolImplementation,
},
Read: {
schema: ReadSchema,
description: ReadDescription.trim(),
impl: read as ToolImplementation,
impl: read as unknown as ToolImplementation,
},
TodoWrite: {
schema: TodoWriteSchema,
description: TodoWriteDescription.trim(),
impl: todo_write as ToolImplementation,
impl: todo_write as unknown as ToolImplementation,
},
Write: {
schema: WriteSchema,
description: WriteDescription.trim(),
impl: write as ToolImplementation,
impl: write as unknown as ToolImplementation,
},
} as const satisfies Record<string, ToolAssets>;