Files
software-mansion__argent/packages/tool-server/test/idle-timeout.test.ts
Ignacy Łątka 112336079e fix: crash recovery review issues from #135 + other findings (#137)
* fix: address crash recovery review issues from #135

- crashShutdown now exits with code 1 (was 0 via shared shutdown())
- reconnect() failure no longer aborts the retry loop
- health monitor timeout 3s → 10s (avoids false positives on busy server)
- ListToolsRequestSchema lets errors propagate (empty list could be cached)
- event emitter catches async listener rejections
- forceExit timer keeps event loop alive (removed .unref())
- stderr error handler no longer writes to stdout
- SIGINT/SIGTERM handlers fall back to process.exit(0) if shutdown is null
- fetchTools() checks res.ok

* fix: clarify stderr error handler comment

* fix: revert stderr handler — stdout fallback is safe

The launcher uses readline + regex matching on stdout, so extra
non-matching lines are harmlessly discarded. Restore the original
stdout fallback for stderr errors.

* fix: add fetch timeout, MCP error handlers, and idle timer on GET /tools

- fetchWithReconnect: wrap each attempt in an AbortController with a
  configurable timeout (ARGENT_FETCH_TIMEOUT_MS, default 30s) so a
  hanging tool-server doesn't wedge the MCP parent indefinitely
- MCP parent (mcp-server.ts): install process.stdout/stderr error
  handlers and uncaughtException/unhandledRejection listeners so an
  EPIPE from a closed Codex transport doesn't crash the process
- GET /tools: touch the idle timer so list_tools polling keeps the
  tool-server alive between tool calls
- Add tests for fetch timeout and idle timer on GET /tools

* chore: update showcase GIF asset

* Revert "chore: update showcase GIF asset"

This reverts commit 2b9379fb05.

* fix: add safety timeout to shutdown and validate FETCH_TIMEOUT_MS

- shutdown() now has a 5s force-exit safety timeout matching
  crashShutdown, preventing hangs if server.close() blocks on a
  lingering connection
- ARGENT_FETCH_TIMEOUT_MS is validated: NaN/0/negative values fall
  back to 30s instead of causing immediate aborts

* fix: remove dead null guard on SIGINT/SIGTERM handlers

shutdown is always assigned (line 57) before the handlers are
registered (line 98-99) in the same synchronous block. The arrow
wrapper is still needed to avoid passing the signal string as exitCode.

* cleanup unneeded bang op

* update comment

* update comment

* prettier

* fix: rewrite fetch-timeout test to not depend on pre-built dist

The test spawned `node dist/cli.js mcp` which requires running
`npm run build:mcp` first. If the dist is stale (e.g. from main),
the old code without the AbortController timeout runs and the test
hangs. Rewrite as an in-process test using a replica of the
fetchWithReconnect pattern against a hanging HTTP server.

* test: add minimal EPIPE → Transport Closed repro

Proves the mechanism: a Node child (mimicking argent mcp) without a
stdout error handler dies on EPIPE when the parent (mimicking Codex)
closes its read end. With the handler, the child survives and the
transport stays usable.

* revert: remove speculative EPIPE stdout/stderr handlers from MCP parent

The EPIPE defense assumed Codex would close its read end of the stdio
pipe while the MCP server is still alive. We can't evidence this from
the logs — the proven root cause is the fetch-timeout hang (repro #3).
Remove the handlers and the EPIPE repro test.

* revert emitter promise error handling

* use universal process timeout value

* remove dubious fixes

* remove unused const

* rev event emitter to main

* revert mcp server to main

* move const to file scope

---------

Co-authored-by: pFornagiel <pawel.fornagiel2@gmail.com>
2026-04-16 13:57:56 +02:00

163 lines
4.4 KiB
TypeScript

import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { createHttpApp, type HttpAppHandle } from "../src/http";
import type { Registry } from "@argent/registry";
function stubRegistry() {
return {
getSnapshot: vi.fn(() => ({ services: new Map(), namespaces: [], tools: [] })),
getTool: vi.fn(() => undefined),
invokeTool: vi.fn(),
} as unknown as Registry;
}
describe("idle timeout", () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it("fires onIdle after configured timeout with no activity", () => {
const onIdle = vi.fn();
const handle = createHttpApp(stubRegistry(), {
idleTimeoutMs: 5 * 60_000,
onIdle,
});
vi.advanceTimersByTime(5 * 60_000);
expect(onIdle).toHaveBeenCalledOnce();
handle.dispose();
});
it("does NOT fire when activity keeps resetting the timer", async () => {
const onIdle = vi.fn();
const registry = stubRegistry();
(registry.getTool as ReturnType<typeof vi.fn>).mockReturnValue({
description: "test",
inputSchema: {},
services: () => ({}),
execute: async () => ({ ok: true }),
});
(registry.invokeTool as ReturnType<typeof vi.fn>).mockResolvedValue({
ok: true,
});
const handle = createHttpApp(registry, {
idleTimeoutMs: 5 * 60_000,
onIdle,
});
const request = await import("supertest").then((m) => m.default);
// Simulate activity every 3 minutes (well within the 5-min timeout)
for (let i = 0; i < 4; i++) {
vi.advanceTimersByTime(3 * 60_000);
await request(handle.app).post("/tools/test-tool").send({}).expect(200);
}
expect(onIdle).not.toHaveBeenCalled();
handle.dispose();
});
it("resets the timer on tool invocation", async () => {
const onIdle = vi.fn();
const registry = stubRegistry();
(registry.getTool as ReturnType<typeof vi.fn>).mockReturnValue({
description: "test",
inputSchema: {},
services: () => ({}),
execute: async () => ({ ok: true }),
});
(registry.invokeTool as ReturnType<typeof vi.fn>).mockResolvedValue({
ok: true,
});
const handle = createHttpApp(registry, {
idleTimeoutMs: 5 * 60_000,
onIdle,
});
const request = await import("supertest").then((m) => m.default);
// Advance 4 minutes (not enough to trigger)
vi.advanceTimersByTime(4 * 60_000);
expect(onIdle).not.toHaveBeenCalled();
// Call a tool — resets the timer
await request(handle.app).post("/tools/test-tool").send({}).expect(200);
// Advance another 4 minutes — still not 5 from last activity
vi.advanceTimersByTime(4 * 60_000);
expect(onIdle).not.toHaveBeenCalled();
// Advance 1 more minute — now 5 since last activity
vi.advanceTimersByTime(1 * 60_000);
expect(onIdle).toHaveBeenCalledOnce();
handle.dispose();
});
it("does not start a timer when timeout is 0", () => {
const onIdle = vi.fn();
const handle = createHttpApp(stubRegistry(), {
idleTimeoutMs: 0,
onIdle,
});
vi.advanceTimersByTime(60 * 60_000);
expect(onIdle).not.toHaveBeenCalled();
handle.dispose();
});
it("does not start a timer when no onIdle callback", () => {
const handle = createHttpApp(stubRegistry(), {
idleTimeoutMs: 5 * 60_000,
});
// Should not throw or fire anything
vi.advanceTimersByTime(10 * 60_000);
handle.dispose();
});
it("clears the timer on dispose()", () => {
const onIdle = vi.fn();
const handle = createHttpApp(stubRegistry(), {
idleTimeoutMs: 5 * 60_000,
onIdle,
});
handle.dispose();
vi.advanceTimersByTime(10 * 60_000);
expect(onIdle).not.toHaveBeenCalled();
});
it("resets the timer on GET /tools (list tools)", async () => {
const onIdle = vi.fn();
const handle = createHttpApp(stubRegistry(), {
idleTimeoutMs: 5 * 60_000,
onIdle,
});
const request = await import("supertest").then((m) => m.default);
// Poll GET /tools every minute — well within the 5-min idle window.
for (let i = 0; i < 6; i++) {
vi.advanceTimersByTime(60_000);
await request(handle.app).get("/tools").expect(200);
}
// 6 minutes have passed total, but the timer should have been
// refreshed each time — so onIdle must NOT fire.
expect(onIdle).not.toHaveBeenCalled();
handle.dispose();
});
});