mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
112336079e
* 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>
163 lines
4.4 KiB
TypeScript
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();
|
|
});
|
|
});
|