Skip to content

Commit 3a1f2ba

Browse files
committed
fix: critical bugs and error handling improvements
- Add custom error classes for DCP command handling (replaces magic strings) - Validate purgeErrors.turns and turnProtection.turns >= 1 (prevents data loss) - Replace silent error swallowing with console.error fallback in logger - Add config validation warning for invalid turns values Fixes: - Magic string error control flow anti-pattern - Potential immediate pruning of all errors with turns=0 - Silent error hiding in logger methods
1 parent 11e3774 commit 3a1f2ba

File tree

5 files changed

+112
-9
lines changed

5 files changed

+112
-9
lines changed

lib/config.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,17 @@ export function validateConfigTypes(config: Record<string, any>): ValidationErro
243243
actual: typeof config.turnProtection.turns,
244244
})
245245
}
246+
// Warn if turns is 0 or negative
247+
if (
248+
typeof config.turnProtection.turns === "number" &&
249+
config.turnProtection.turns < 1
250+
) {
251+
errors.push({
252+
key: "turnProtection.turns",
253+
expected: "positive number (>= 1)",
254+
actual: `${config.turnProtection.turns}`,
255+
})
256+
}
246257
}
247258

248259
// Commands validator
@@ -497,6 +508,17 @@ export function validateConfigTypes(config: Record<string, any>): ValidationErro
497508
actual: typeof strategies.purgeErrors.turns,
498509
})
499510
}
511+
// Warn if turns is 0 or negative - will be clamped to 1
512+
if (
513+
typeof strategies.purgeErrors.turns === "number" &&
514+
strategies.purgeErrors.turns < 1
515+
) {
516+
errors.push({
517+
key: "strategies.purgeErrors.turns",
518+
expected: "positive number (>= 1)",
519+
actual: `${strategies.purgeErrors.turns} (will be clamped to 1)`,
520+
})
521+
}
500522
if (
501523
strategies.purgeErrors.protectedTools !== undefined &&
502524
!Array.isArray(strategies.purgeErrors.protectedTools)

lib/errors.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* Custom error classes for DCP plugin.
3+
* Used for control flow in command handling to avoid magic string comparisons.
4+
*/
5+
6+
/**
7+
* Base class for DCP command handling errors.
8+
* These are thrown to signal that a command was handled and no further processing is needed.
9+
*/
10+
export class DCPCommandHandledError extends Error {
11+
constructor(message: string = "DCP command handled") {
12+
super(message)
13+
this.name = "DCPCommandHandledError"
14+
}
15+
}
16+
17+
/**
18+
* Specific error types for different command outcomes.
19+
*/
20+
export class DCPContextHandledError extends DCPCommandHandledError {
21+
constructor() {
22+
super("Context command handled")
23+
this.name = "DCPContextHandledError"
24+
}
25+
}
26+
27+
export class DCPStatsHandledError extends DCPCommandHandledError {
28+
constructor() {
29+
super("Stats command handled")
30+
this.name = "DCPStatsHandledError"
31+
}
32+
}
33+
34+
export class DCPSweepHandledError extends DCPCommandHandledError {
35+
constructor() {
36+
super("Sweep command handled")
37+
this.name = "DCPSweepHandledError"
38+
}
39+
}
40+
41+
export class DCPManualHandledError extends DCPCommandHandledError {
42+
constructor() {
43+
super("Manual command handled")
44+
this.name = "DCPManualHandledError"
45+
}
46+
}
47+
48+
export class DCPManualTriggerBlockedError extends DCPCommandHandledError {
49+
constructor() {
50+
super("Manual trigger blocked")
51+
this.name = "DCPManualTriggerBlockedError"
52+
}
53+
}
54+
55+
export class DCPHelpHandledError extends DCPCommandHandledError {
56+
constructor() {
57+
super("Help command handled")
58+
this.name = "DCPHelpHandledError"
59+
}
60+
}
61+
62+
/**
63+
* Type guard to check if an error is a DCP command handled error.
64+
*/
65+
export function isDCPCommandHandledError(error: unknown): error is DCPCommandHandledError {
66+
return error instanceof DCPCommandHandledError
67+
}

lib/hooks.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ import { handleSweepCommand } from "./commands/sweep"
1414
import { handleManualToggleCommand, handleManualTriggerCommand } from "./commands/manual"
1515
import { ensureSessionInitialized } from "./state/state"
1616
import { getCurrentParams } from "./strategies/utils"
17+
import {
18+
DCPContextHandledError,
19+
DCPStatsHandledError,
20+
DCPSweepHandledError,
21+
DCPManualHandledError,
22+
DCPManualTriggerBlockedError,
23+
DCPHelpHandledError,
24+
} from "./errors"
1725

1826
const INTERNAL_AGENT_SIGNATURES = [
1927
"You are a title generator",
@@ -172,12 +180,12 @@ export function createCommandExecuteHandler(
172180

173181
if (subcommand === "context") {
174182
await handleContextCommand(commandCtx)
175-
throw new Error("__DCP_CONTEXT_HANDLED__")
183+
throw new DCPContextHandledError()
176184
}
177185

178186
if (subcommand === "stats") {
179187
await handleStatsCommand(commandCtx)
180-
throw new Error("__DCP_STATS_HANDLED__")
188+
throw new DCPStatsHandledError()
181189
}
182190

183191
if (subcommand === "sweep") {
@@ -186,12 +194,12 @@ export function createCommandExecuteHandler(
186194
args: subArgs,
187195
workingDirectory,
188196
})
189-
throw new Error("__DCP_SWEEP_HANDLED__")
197+
throw new DCPSweepHandledError()
190198
}
191199

192200
if (subcommand === "manual") {
193201
await handleManualToggleCommand(commandCtx, subArgs[0]?.toLowerCase())
194-
throw new Error("__DCP_MANUAL_HANDLED__")
202+
throw new DCPManualHandledError()
195203
}
196204

197205
if (
@@ -201,7 +209,7 @@ export function createCommandExecuteHandler(
201209
const userFocus = subArgs.join(" ").trim()
202210
const prompt = await handleManualTriggerCommand(commandCtx, subcommand, userFocus)
203211
if (!prompt) {
204-
throw new Error("__DCP_MANUAL_TRIGGER_BLOCKED__")
212+
throw new DCPManualTriggerBlockedError()
205213
}
206214

207215
state.pendingManualTrigger = {
@@ -218,7 +226,7 @@ export function createCommandExecuteHandler(
218226
}
219227

220228
await handleHelpCommand(commandCtx)
221-
throw new Error("__DCP_HELP_HANDLED__")
229+
throw new DCPHelpHandledError()
222230
}
223231
}
224232
}

lib/logger.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ export class Logger {
8585

8686
const logFile = join(dailyLogDir, `${new Date().toISOString().split("T")[0]}.log`)
8787
await writeFile(logFile, logLine, { flag: "a" })
88-
} catch (error) {}
88+
} catch (error) {
89+
// Fallback to console if file logging fails
90+
console.error("[DCP Logger] Failed to write log:", error)
91+
}
8992
}
9093

9194
info(message: string, data?: any) {
@@ -206,6 +209,8 @@ export class Logger {
206209
const timestamp = new Date().toISOString().replace(/[:.]/g, "-")
207210
const contextFile = join(contextDir, `${timestamp}.json`)
208211
await writeFile(contextFile, JSON.stringify(minimized, null, 2))
209-
} catch (error) {}
212+
} catch (error) {
213+
console.error("[DCP Logger] Failed to save context:", error)
214+
}
210215
}
211216
}

lib/strategies/purge-errors.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ export const purgeErrors = (
3939
}
4040

4141
const protectedTools = config.strategies.purgeErrors.protectedTools
42-
const turnThreshold = config.strategies.purgeErrors.turns
42+
// Ensure turnThreshold is at least 1 to prevent immediate pruning of all errors
43+
const turnThreshold = Math.max(1, config.strategies.purgeErrors.turns)
4344

4445
const newPruneIds: string[] = []
4546

0 commit comments

Comments
 (0)