NotAShelf/troutbot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: add defensive checks; improve code quality

Browse source 
Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: I8c88c07bd852c78d196705da03f372e06a6a6964
This commit is contained in:
raf 2026-02-07 16:13:14 +03:00
commit 38105ec09c
Signed by: NotAShelf
GPG key ID: 29D95B64378DB4BF
4 changed files with 255 additions and 112 deletions
Download patch file Download diff file Expand all files Collapse all files

103
src/config.ts
View file

@ -1,12 +1,10 @@
import fs from 'node:fs';
import path from 'node:path';
import { createJiti } from 'jiti';
import dotenv from 'dotenv';
import type { Config } from './types.js';
import fs from "node:fs";
import path from "node:path";
import dotenv from "dotenv";
import type { Config } from "./types.js";
dotenv.config();
const jiti = createJiti(__filename, { interopDefault: true });
// Suppress dotenv warnings
dotenv.config({ quiet: true });
const defaults: Config = {
server: { port: 3000 },
@ -34,31 +32,34 @@ const defaults: Config = {
includeReasoning: true,
messages: {
positive: [
'This {type} looks great for the trout! All signals point upstream.',
'The trout approve of this {type}. Swim on!',
'Splashing good news - this {type} is looking healthy.',
"This {type} looks great for the trout! All signals point upstream.",
"The trout approve of this {type}. Swim on!",
"Splashing good news - this {type} is looking healthy.",
],
negative: [
'This {type} is muddying the waters. The trout are concerned.',
'Warning: the trout sense trouble in this {type}.',
'Something smells fishy about this {type}. Please review.',
"This {type} is muddying the waters. The trout are concerned.",
"Warning: the trout sense trouble in this {type}.",
"Something smells fishy about this {type}. Please review.",
],
neutral: [
'The trout have no strong feelings about this {type}.',
'This {type} is neither upstream nor downstream. Neutral waters.',
'The trout are watching this {type} with mild interest.',
"The trout have no strong feelings about this {type}.",
"This {type} is neither upstream nor downstream. Neutral waters.",
"The trout are watching this {type} with mild interest.",
],
},
commentMarker: '<!-- troutbot -->',
commentMarker: "<!-- troutbot -->",
allowUpdates: false,
},
logging: {
level: 'info',
file: 'troutbot.log',
level: "info",
file: "troutbot.log",
},
};
export function deepMerge<T extends Record<string, unknown>>(target: T, source: Partial<T>): T {
export function deepMerge<T extends Record<string, unknown>>(
target: T,
source: Partial<T>,
): T {
const result = { ...target };
for (const key of Object.keys(source) as (keyof T)[]) {
const sourceVal = source[key];
@ -66,15 +67,15 @@ export function deepMerge<T extends Record<string, unknown>>(target: T, source:
if (
sourceVal !== null &&
sourceVal !== undefined &&
typeof sourceVal === 'object' &&
typeof sourceVal === "object" &&
!Array.isArray(sourceVal) &&
typeof targetVal === 'object' &&
typeof targetVal === "object" &&
!Array.isArray(targetVal) &&
targetVal !== null
) {
result[key] = deepMerge(
targetVal as Record<string, unknown>,
sourceVal as Record<string, unknown>
sourceVal as Record<string, unknown>,
) as T[keyof T];
} else if (sourceVal !== undefined) {
result[key] = sourceVal as T[keyof T];
@ -83,30 +84,35 @@ export function deepMerge<T extends Record<string, unknown>>(target: T, source:
return result;
}
export function loadConfig(): Config {
const configPath = process.env.CONFIG_PATH || 'config.ts';
export async function loadConfig(): Promise<Config> {
const configPath = process.env.CONFIG_PATH || "config.ts";
const resolvedPath = path.resolve(configPath);
let fileConfig: Partial<Config> = {};
if (fs.existsSync(resolvedPath)) {
const loaded = jiti(resolvedPath) as Partial<Config> | { default: Partial<Config> };
fileConfig = 'default' in loaded ? loaded.default : loaded;
const loaded = await import(resolvedPath);
fileConfig =
"default" in loaded
? loaded.default
: (loaded as unknown as Partial<Config>);
} else if (process.env.CONFIG_PATH) {
console.warn(
`Warning: CONFIG_PATH is set to "${process.env.CONFIG_PATH}" but file not found at ${resolvedPath}`
`Warning: CONFIG_PATH is set to "${process.env.CONFIG_PATH}" but file not found at ${resolvedPath}`,
);
}
const config = deepMerge(
defaults as unknown as Record<string, unknown>,
fileConfig as unknown as Record<string, unknown>
fileConfig as unknown as Record<string, unknown>,
) as unknown as Config;
// Environment variable overrides
if (process.env.PORT) {
const parsed = parseInt(process.env.PORT, 10);
if (Number.isNaN(parsed)) {
throw new Error(`Invalid PORT value: "${process.env.PORT}" is not a number`);
throw new Error(
`Invalid PORT value: "${process.env.PORT}" is not a number`,
);
}
config.server.port = parsed;
}
@ -119,7 +125,7 @@ export function loadConfig(): Config {
config.dashboard = {
...(config.dashboard || { enabled: true }),
auth: {
type: 'token',
type: "token",
token: process.env.DASHBOARD_TOKEN,
},
};
@ -129,18 +135,18 @@ export function loadConfig(): Config {
config.dashboard = {
...(config.dashboard || { enabled: true }),
auth: {
type: 'basic',
type: "basic",
username: process.env.DASHBOARD_USERNAME,
password: process.env.DASHBOARD_PASSWORD,
},
};
}
const validLogLevels = ['debug', 'info', 'warn', 'error'];
const validLogLevels = ["debug", "info", "warn", "error"];
if (process.env.LOG_LEVEL) {
if (!validLogLevels.includes(process.env.LOG_LEVEL)) {
throw new Error(
`Invalid LOG_LEVEL: "${process.env.LOG_LEVEL}". Must be one of: ${validLogLevels.join(', ')}`
`Invalid LOG_LEVEL: "${process.env.LOG_LEVEL}". Must be one of: ${validLogLevels.join(", ")}`,
);
}
config.logging.level = process.env.LOG_LEVEL;
@ -151,23 +157,36 @@ export function loadConfig(): Config {
}
export function validate(config: Config): void {
if (!config.server.port || config.server.port < 1 || config.server.port > 65535) {
throw new Error('Invalid server port');
if (
!config.server.port ||
config.server.port < 1 ||
config.server.port > 65535
) {
throw new Error("Invalid server port");
}
const { backends } = config.engine;
if (!backends.checks.enabled && !backends.diff.enabled && !backends.quality.enabled) {
throw new Error('At least one engine backend must be enabled');
if (
!backends.checks.enabled &&
!backends.diff.enabled &&
!backends.quality.enabled
) {
throw new Error("At least one engine backend must be enabled");
}
const { weights } = config.engine;
for (const [key, value] of Object.entries(weights)) {
if (value < 0) {
throw new Error(`Backend weight "${key}" must be non-negative, got ${value}`);
throw new Error(
`Backend weight "${key}" must be non-negative, got ${value}`,
);
}
}
if (config.engine.confidenceThreshold < 0 || config.engine.confidenceThreshold > 1) {
throw new Error('confidenceThreshold must be between 0 and 1');
if (
config.engine.confidenceThreshold < 0 ||
config.engine.confidenceThreshold > 1
) {
throw new Error("confidenceThreshold must be between 0 and 1");
}
}

34
src/filters.ts
View file

@ -1,41 +1,53 @@
import type { FiltersConfig, WebhookEvent } from './types.js';
import type { FiltersConfig, WebhookEvent } from "./types.js";
export function shouldProcess(
event: WebhookEvent,
filters: FiltersConfig
filters: FiltersConfig,
): { pass: boolean; reason?: string } {
// Label filters
if (filters.labels.include.length > 0) {
const hasRequired = event.labels.some((l) => filters.labels.include.includes(l));
const hasRequired = event.labels.some((l) =>
filters.labels.include.includes(l),
);
if (!hasRequired) {
return { pass: false, reason: 'Missing required label' };
return { pass: false, reason: "Missing required label" };
}
}
if (filters.labels.exclude.length > 0) {
const hasExcluded = event.labels.some((l) => filters.labels.exclude.includes(l));
const hasExcluded = event.labels.some((l) =>
filters.labels.exclude.includes(l),
);
if (hasExcluded) {
return { pass: false, reason: 'Has excluded label' };
return { pass: false, reason: "Has excluded label" };
}
}
// Author filters
if (filters.authors.include && filters.authors.include.length > 0) {
if (!filters.authors.include.includes(event.author)) {
return { pass: false, reason: 'Author not in include list' };
const normalizedAuthor = event.author.toLowerCase();
const hasIncluded = filters.authors.include.some(
(a) => a.toLowerCase() === normalizedAuthor,
);
if (!hasIncluded) {
return { pass: false, reason: "Author not in include list" };
}
}
if (filters.authors.exclude.length > 0) {
if (filters.authors.exclude.includes(event.author)) {
return { pass: false, reason: 'Author is excluded' };
const normalizedAuthor = event.author.toLowerCase();
const isExcluded = filters.authors.exclude.some(
(a) => a.toLowerCase() === normalizedAuthor,
);
if (isExcluded) {
return { pass: false, reason: "Author is excluded" };
}
}
// Branch filters (PRs only)
if (event.branch && filters.branches.include.length > 0) {
if (!filters.branches.include.includes(event.branch)) {
return { pass: false, reason: 'Branch not in include list' };
return { pass: false, reason: "Branch not in include list" };
}
}

191
src/github.ts
View file

@ -1,12 +1,14 @@
import { Octokit } from '@octokit/rest';
import { getLogger } from './logger.js';
import type { CheckRun, PRFile, ResponseConfig } from './types.js';
import { Octokit } from "@octokit/rest";
import { getLogger } from "./logger.js";
import type { CheckRun, PRFile } from "./types.js";
let octokit: Octokit | null = null;
export function initGitHub(token?: string): void {
if (!token) {
getLogger().warn('No GITHUB_TOKEN set - running in dry-run mode, comments will not be posted');
getLogger().warn(
"No GITHUB_TOKEN set - running in dry-run mode, comments will not be posted",
);
return;
}
octokit = new Octokit({ auth: token });
@ -21,13 +23,20 @@ export async function postComment(
owner: string,
repo: string,
issueNumber: number,
body: string
body: string,
): Promise<void> {
if (!octokit) {
getLogger().info(`[dry-run] Would post comment on ${owner}/${repo}#${issueNumber}:\n${body}`);
getLogger().info(
`[dry-run] Would post comment on ${owner}/${repo}#${issueNumber}:\n${body}`,
);
return;
}
await octokit.issues.createComment({ owner, repo, issue_number: issueNumber, body });
await octokit.issues.createComment({
owner,
repo,
issue_number: issueNumber,
body,
});
getLogger().info(`Posted comment on ${owner}/${repo}#${issueNumber}`);
}
@ -35,7 +44,7 @@ export async function hasExistingComment(
owner: string,
repo: string,
issueNumber: number,
marker: string
marker: string,
): Promise<{ exists: boolean; commentId?: number }> {
if (!octokit) {
return { exists: false };
@ -59,13 +68,18 @@ export async function updateComment(
owner: string,
repo: string,
commentId: number,
body: string
body: string,
): Promise<void> {
if (!octokit) {
getLogger().info(`[dry-run] Would update comment ${commentId}:\n${body}`);
return;
}
await octokit.issues.updateComment({ owner, repo, comment_id: commentId, body });
await octokit.issues.updateComment({
owner,
repo,
comment_id: commentId,
body,
});
getLogger().info(`Updated comment ${commentId} on ${owner}/${repo}`);
}
@ -74,26 +88,41 @@ export async function createReaction(
repo: string,
commentId: number,
reaction:
| 'thumbs_up'
| 'thumbs_down'
| 'laugh'
| 'confused'
| 'heart'
| 'hooray'
| 'eyes'
| 'rocket'
| "thumbs_up"
| "thumbs_down"
| "laugh"
| "confused"
| "heart"
| "hooray"
| "eyes"
| "rocket",
): Promise<void> {
if (!octokit) {
getLogger().info(`[dry-run] Would add ${reaction} reaction to comment ${commentId}`);
getLogger().info(
`[dry-run] Would add ${reaction} reaction to comment ${commentId}`,
);
return;
}
// Map thumbs_up/thumbs_down to GitHub API format (+1/-1)
const content = reaction === 'thumbs_up' ? '+1' : reaction === 'thumbs_down' ? '-1' : reaction;
const content =
reaction === "thumbs_up"
? "+1"
: reaction === "thumbs_down"
? "-1"
: reaction;
await octokit.reactions.createForIssueComment({
owner,
repo,
comment_id: commentId,
content: content as '+1' | '-1' | 'laugh' | 'confused' | 'heart' | 'hooray' | 'eyes' | 'rocket',
content: content as
| "+1"
| "-1"
| "laugh"
| "confused"
| "heart"
| "hooray"
| "eyes"
| "rocket",
});
getLogger().info(`Added ${reaction} reaction to comment ${commentId}`);
}
@ -102,10 +131,10 @@ export async function createReaction(
export async function fetchCheckRuns(
owner: string,
repo: string,
ref: string
ref: string,
): Promise<CheckRun[]> {
if (!octokit) {
getLogger().debug('[dry-run] Cannot fetch check runs without a token');
getLogger().debug("[dry-run] Cannot fetch check runs without a token");
return [];
}
@ -126,10 +155,10 @@ export async function fetchCheckRuns(
export async function fetchPRFiles(
owner: string,
repo: string,
prNumber: number
prNumber: number,
): Promise<PRFile[]> {
if (!octokit) {
getLogger().debug('[dry-run] Cannot fetch PR files without a token');
getLogger().debug("[dry-run] Cannot fetch PR files without a token");
return [];
}
@ -151,7 +180,7 @@ export async function fetchPRFiles(
export async function fetchPR(
owner: string,
repo: string,
prNumber: number
prNumber: number,
): Promise<{
title: string;
body: string;
@ -163,12 +192,18 @@ export async function fetchPR(
if (!octokit) return null;
try {
const { data } = await octokit.pulls.get({ owner, repo, pull_number: prNumber });
const { data } = await octokit.pulls.get({
owner,
repo,
pull_number: prNumber,
});
return {
title: data.title,
body: data.body || '',
author: data.user?.login || '',
labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')),
body: data.body || "",
author: data.user?.login || "",
labels: (data.labels || []).map((l) =>
typeof l === "string" ? l : l.name || "",
),
branch: data.head.ref,
sha: data.head.sha,
};
@ -181,7 +216,7 @@ export async function fetchPR(
export async function fetchIssue(
owner: string,
repo: string,
issueNumber: number
issueNumber: number,
): Promise<{
title: string;
body: string;
@ -191,19 +226,57 @@ export async function fetchIssue(
if (!octokit) return null;
try {
const { data } = await octokit.issues.get({ owner, repo, issue_number: issueNumber });
const { data } = await octokit.issues.get({
owner,
repo,
issue_number: issueNumber,
});
return {
title: data.title,
body: data.body || '',
author: data.user?.login || '',
labels: (data.labels || []).map((l) => (typeof l === 'string' ? l : l.name || '')),
body: data.body || "",
author: data.user?.login || "",
labels: (data.labels || []).map((l) =>
typeof l === "string" ? l : l.name || "",
),
};
} catch (err) {
getLogger().debug(`Failed to fetch issue ${owner}/${repo}#${issueNumber}`, err);
getLogger().debug(
`Failed to fetch issue ${owner}/${repo}#${issueNumber}`,
err,
);
return null;
}
}
export async function listAccessibleRepositories(): Promise<
Array<{ owner: string; repo: string }>
> {
if (!octokit) {
getLogger().debug("[dry-run] Cannot fetch repositories without a token");
return [];
}
const repos: Array<{ owner: string; repo: string }> = [];
for await (const response of octokit.paginate.iterator(
octokit.repos.listForAuthenticatedUser,
{
per_page: 100,
sort: "updated",
},
)) {
for (const repo of response.data) {
if (!repo.full_name || typeof repo.full_name !== "string") continue;
const parts = repo.full_name.split("/");
if (parts.length < 2 || !parts[0] || !parts[1]) continue;
const [owner, repoName] = parts;
repos.push({ owner, repo: repoName });
}
}
return repos;
}
export interface RecentComment {
id: number;
body: string;
@ -216,10 +289,10 @@ export interface RecentComment {
export async function listRecentComments(
owner: string,
repo: string,
since: Date
since: Date,
): Promise<RecentComment[]> {
if (!octokit) {
getLogger().debug('[dry-run] Cannot fetch comments without a token');
getLogger().debug("[dry-run] Cannot fetch comments without a token");
return [];
}
@ -227,12 +300,15 @@ export async function listRecentComments(
const comments: RecentComment[] = [];
// Fetch recent issue comments
const issueComments = await octokit.paginate(octokit.issues.listCommentsForRepo, {
const issueComments = await octokit.paginate(
octokit.issues.listCommentsForRepo,
{
owner,
repo,
since: sinceIso,
per_page: 100,
});
},
);
for (const comment of issueComments) {
if (!comment.body) continue;
@ -240,9 +316,11 @@ export async function listRecentComments(
comments.push({
id: comment.id,
body: comment.body,
author: comment.user?.login || '',
author: comment.user?.login || "",
createdAt: comment.created_at,
issueNumber: comment.issue_url ? parseInt(comment.issue_url.split('/').pop() || '0', 10) : 0,
issueNumber: comment.issue_url
? parseInt(comment.issue_url.split("/").pop() || "0", 10)
: 0,
isPullRequest: false, // we'll determine this by fetching the issue
});
}
@ -256,19 +334,28 @@ function pickRandom(list: string[]): string {
}
export function formatComment(
responseConfig: ResponseConfig,
type: 'issue' | 'pull_request',
responseConfig: {
commentMarker: string;
includeConfidence: boolean;
includeReasoning: boolean;
messages: {
positive: string[];
negative: string[];
neutral: string[];
};
},
type: "issue" | "pull_request",
impact: string,
confidence: number,
reasoning: string
reasoning: string,
): string {
const typeLabel = type === 'pull_request' ? 'pull request' : 'issue';
const typeLabel = type === "pull_request" ? "pull request" : "issue";
const { messages } = responseConfig;
let messageList: string[];
if (impact === 'positive') {
if (impact === "positive") {
messageList = messages.positive;
} else if (impact === 'negative') {
} else if (impact === "negative") {
messageList = messages.negative;
} else {
messageList = messages.neutral;
@ -276,14 +363,16 @@ export function formatComment(
const template = pickRandom(messageList);
let body = responseConfig.commentMarker + '\n\n';
body += template.replace(/\{type\}/g, typeLabel).replace(/\{impact\}/g, impact);
let body = responseConfig.commentMarker + "\n\n";
body += template
.replace(/\{type\}/g, typeLabel)
.replace(/\{impact\}/g, impact);
if (responseConfig.includeConfidence) {
body += `\n\n**Confidence:** ${(confidence * 100).toFixed(0)}%`;
}
if (responseConfig.includeReasoning) {
body += `\n\n**Analysis:** ${reasoning}`;
body += `\n\n**Analysis:**\n\`\`\`\n${reasoning}\n\`\`\``;
}
return body;

31
src/types.ts
View file

@ -16,7 +16,7 @@ export interface PollingConfig {
backfill?: boolean;
stateFile?: string;
authorizedUsers?: string[];
repositories?: RepoConfig[];
repositories?: RepoPattern[]; // Can include wildcards like [{ owner: 'NotAShelf', repo: '*' }]
}
export interface ServerConfig {
@ -31,7 +31,7 @@ export interface DashboardConfig {
}
export interface DashboardAuthConfig {
type: 'basic' | 'token';
type: "basic" | "token";
username?: string;
password?: string;
token?: string;
@ -42,6 +42,11 @@ export interface RepoConfig {
repo: string;
}
export interface RepoPattern {
owner: string;
repo?: string; // undefined means all repos for that owner
}
export interface FiltersConfig {
labels: {
include: string[];
@ -106,12 +111,30 @@ export interface LoggingConfig {
file: string;
}
export type Impact = 'positive' | 'negative' | 'neutral';
export type Impact = "positive" | "negative" | "neutral";
export interface AnalysisResult {
impact: Impact;
confidence: number;
reasoning: string;
// Multi-dimensional scoring for sophisticated analysis
dimensions?: {
correctness: number; // -1 to 1: will this work correctly?
risk: number; // -1 to 1: could this break things? (negative = risky)
maintainability: number; // -1 to 1: will this be maintainable?
alignment: number; // -1 to 1: does this fit the project?
};
// Correlation analysis
correlations?: {
suspiciousPatterns: string[];
reinforcingSignals: string[];
contradictions: string[];
};
// Uncertainty quantification
uncertainty?: {
confidenceInterval: [number, number]; // [lower, upper]
primaryUncertaintySource: string;
};
}
export interface EngineBackend {
@ -121,7 +144,7 @@ export interface EngineBackend {
export interface WebhookEvent {
action: string;
type: 'issue' | 'pull_request';
type: "issue" | "pull_request";
number: number;
title: string;
body: string;