fix(tools): clear timeout timers and update audit state
This commit is contained in:
@@ -3,6 +3,13 @@
|
|||||||
Date: 2026-02-16
|
Date: 2026-02-16
|
||||||
Scope: Production-risk-first audit of bugs, code improvements, and feature opportunities.
|
Scope: Production-risk-first audit of bugs, code improvements, and feature opportunities.
|
||||||
|
|
||||||
|
## Remediation Status (2026-02-16)
|
||||||
|
|
||||||
|
- ✅ F-001 addressed: chat markdown rendering now sanitizes HTML before DOM insertion in `src/gateway/ui/pages/chat.js` (and legacy `src/gateway/ui/chat.html`).
|
||||||
|
- ✅ F-006 addressed: inbound HTTP request bodies now enforce a configurable max-size limit (`server.max_request_body_bytes`) with `413 Payload Too Large` responses.
|
||||||
|
- ✅ F-007 addressed: `ToolExecutor` timeout timer handles are now cleared in `finally`, preventing orphan timers on fast/failed tool calls.
|
||||||
|
- ✅ F-016 partially addressed: gateway + webhook body readers were consolidated into shared utility `src/utils/httpBody.ts` with size-limit enforcement.
|
||||||
|
|
||||||
## Executive Summary
|
## Executive Summary
|
||||||
|
|
||||||
Current health snapshot:
|
Current health snapshot:
|
||||||
|
|||||||
+572
-487
File diff suppressed because it is too large
Load Diff
@@ -1,4 +1,4 @@
|
|||||||
import { describe, it, expect } from 'vitest';
|
import { describe, it, expect, vi } from 'vitest';
|
||||||
import { ToolExecutor } from './executor.js';
|
import { ToolExecutor } from './executor.js';
|
||||||
import { ToolRegistry } from './registry.js';
|
import { ToolRegistry } from './registry.js';
|
||||||
import { HookEngine } from '../hooks/engine.js';
|
import { HookEngine } from '../hooks/engine.js';
|
||||||
@@ -98,6 +98,22 @@ describe('ToolExecutor', () => {
|
|||||||
expect(result.output).toContain('[truncated]');
|
expect(result.output).toContain('[truncated]');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('clears timeout timer after fast tool completion', async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
try {
|
||||||
|
const registry = new ToolRegistry();
|
||||||
|
registry.register(echoTool);
|
||||||
|
const hooks = new HookEngine({ confirm: [], log: [], silent: [] });
|
||||||
|
const executor = new ToolExecutor(registry, hooks, { defaultTimeoutMs: 30_000 });
|
||||||
|
|
||||||
|
const result = await executor.execute('test.echo', { text: 'hello' });
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
expect(vi.getTimerCount()).toBe(0);
|
||||||
|
} finally {
|
||||||
|
vi.useRealTimers();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it('blocks on confirm hook and resolves when approved', async () => {
|
it('blocks on confirm hook and resolves when approved', async () => {
|
||||||
const registry = new ToolRegistry();
|
const registry = new ToolRegistry();
|
||||||
registry.register(echoTool);
|
registry.register(echoTool);
|
||||||
|
|||||||
+11
-3
@@ -224,6 +224,7 @@ export class ToolExecutor {
|
|||||||
agent_tier: context?.tier,
|
agent_tier: context?.tier,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
let timeoutHandle: NodeJS.Timeout | undefined;
|
||||||
try {
|
try {
|
||||||
const result = await Promise.race([
|
const result = await Promise.race([
|
||||||
(async () => {
|
(async () => {
|
||||||
@@ -239,9 +240,12 @@ export class ToolExecutor {
|
|||||||
}
|
}
|
||||||
return tool.execute(args);
|
return tool.execute(args);
|
||||||
})(),
|
})(),
|
||||||
new Promise<ToolResult>((_, reject) =>
|
new Promise<ToolResult>((_, reject) => {
|
||||||
setTimeout(() => reject(new Error(`Tool '${toolName}' timed out after ${this.defaultTimeoutMs}ms`)), this.defaultTimeoutMs),
|
timeoutHandle = setTimeout(
|
||||||
),
|
() => reject(new Error(`Tool '${toolName}' timed out after ${this.defaultTimeoutMs}ms`)),
|
||||||
|
this.defaultTimeoutMs,
|
||||||
|
);
|
||||||
|
}),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
const duration = Date.now() - startTime;
|
const duration = Date.now() - startTime;
|
||||||
@@ -286,6 +290,10 @@ export class ToolExecutor {
|
|||||||
output: '',
|
output: '',
|
||||||
error: String(errorRedaction.value),
|
error: String(errorRedaction.value),
|
||||||
};
|
};
|
||||||
|
} finally {
|
||||||
|
if (timeoutHandle) {
|
||||||
|
clearTimeout(timeoutHandle);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user