Skip to content

fix: 10min idle timeout + poll stdin for dead clients (#148) #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
justrach merged 3 commits into from
Apr 10, 2026
Merged

fix: 10min idle timeout + poll stdin for dead clients (#148) #233

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8754cf1
fix: 10min idle timeout + poll stdin for dead clients (#148)
justrach Apr 6, 2026
252a9f1
test: add unit tests for timeout + POLLHUP detection (#148)
justrach Apr 6, 2026
051d6c8
Merge remote-tracking branch 'origin/main' into fix/148-timeout
justrach Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions src/tests.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4824,6 +4824,66 @@ test "issue-148: MCP session survives 2-minute idle" {
try testing.expect(now - last > 2 * 60 * 1000);
}

test "issue-148: open pipe does not trigger HUP" {
const pipe = try std.posix.pipe();
defer std.posix.close(pipe[0]);
defer std.posix.close(pipe[1]);

var poll_fds = [_]std.posix.pollfd{.{
.fd = pipe[0],
.events = std.posix.POLL.IN | std.posix.POLL.HUP,
.revents = 0,
}};

const result = try std.posix.poll(&poll_fds, 0);
try testing.expectEqual(@as(usize, 0), result);
}

test "issue-148: codedb mcp exits when stdin is closed" {
// Integration test: spawn codedb mcp, close stdin, verify it exits
var child = std.process.Child.init(
&.{ "zig", "build", "run", "--", "--mcp" },
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove build latency from the 15s MCP shutdown assertion

This integration test launches zig build run -- --mcp, so compile/link startup time is included in elapsed before the < 15_000 assertion. On slower or cold-cache environments, build time alone can exceed 15s even when stdin-HUP shutdown works correctly, making the test flaky and failing for reasons unrelated to watchdog behavior.

Useful? React with 👍 / 👎.

testing.allocator,
);
child.stdin_behavior = .Pipe;
child.stdout_behavior = .Pipe;
child.stderr_behavior = .Ignore;

try child.spawn();

// Send initialize then close stdin (simulate client crash)
const init_msg = "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\",\"params\":{\"protocolVersion\":\"2024-11-05\",\"capabilities\":{},\"clientInfo\":{\"name\":\"test\",\"version\":\"1\"}}}";
const header = std.fmt.comptimePrint("Content-Length: {d}\r\n\r\n", .{init_msg.len});

if (child.stdin) |stdin| {
stdin.writeAll(header) catch {};
stdin.writeAll(init_msg) catch {};
// Close stdin — simulates client disconnecting
stdin.close();
child.stdin = null;
}

// Wait up to 15 seconds for the process to exit
// (watchdog polls every 10s, so it should detect POLLHUP within ~10s)
const start = std.time.milliTimestamp();
const term = child.wait() catch {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add a bounded wait when asserting MCP exit on stdin close

child.wait() blocks indefinitely, so if the MCP process fails to exit after stdin is closed, this test hangs the entire suite until an external timeout kills it. The test comment says it waits “up to 15 seconds,” but there is no actual timeout enforcement before calling wait(), which means this regression test cannot fail fast when the shutdown path breaks.

Useful? React with 👍 / 👎.

// If wait fails, the process is stuck — test fails
try testing.expect(false);
return;
};

const elapsed = std.time.milliTimestamp() - start;

// Should have exited (not been killed by us)
switch (term) {
.Exited => |code| _ = code,
else => {},
Comment on lines +4878 to +4880
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert successful MCP termination in stdin-close test

The new integration test accepts any termination outcome because the switch ignores non-Exited cases and also discards the exit code, so crashes or non-zero exits (for example from a panic or failed zig build run) still satisfy the test as long as the process returns quickly. This can mask regressions in the stdin-HUP shutdown path instead of proving a clean exit.

Useful? React with 👍 / 👎.

}

// Should exit within 15 seconds (10s poll interval + margin)
try testing.expect(elapsed < 15_000);
}

const MmapTrigramIndex = @import("index.zig").MmapTrigramIndex;
const AnyTrigramIndex = @import("index.zig").AnyTrigramIndex;

Expand Down
Loading