From c7a54267b32eb6ab08665d176f7a64282b8863c9 Mon Sep 17 00:00:00 2001 From: yxlyx <85774423+yxlyx@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:07:34 +0800 Subject: [PATCH 1/2] fix(tui): recover poisoned TLS client generations A request-construction TLS failure occurs before the existing connection poison can run, so retries and later TUI trajectories kept reusing a launch-scoped client that only process replacement could heal. Route model HTTP calls through leased generations, rotate on construction failure without deinitializing in-flight users, and preserve the original client for unrelated launch consumers. Gate every managed constructor on CA readiness, distinguish CA warm and request-construction failures in traces, and cover concurrent stale reports plus later ordinary and child POST recovery with a loopback regression. Co-Authored-By: Codegraff --- ...l-http-client-recovery-uses-generations.md | 35 +++ docs/adr/README.md | 1 + src/agent_request.zig | 15 +- src/agent_stream.zig | 13 +- src/http.zig | 37 ++- src/http_client.zig | 275 ++++++++++++++++++ src/http_client_integration_tests.zig | 67 +++++ src/http_warm.zig | 8 +- src/main.zig | 29 +- 9 files changed, 430 insertions(+), 50 deletions(-) create mode 100644 docs/adr/0042-model-http-client-recovery-uses-generations.md create mode 100644 src/http_client.zig create mode 100644 src/http_client_integration_tests.zig diff --git a/docs/adr/0042-model-http-client-recovery-uses-generations.md b/docs/adr/0042-model-http-client-recovery-uses-generations.md new file mode 100644 index 00000000..08d19ae2 --- /dev/null +++ b/docs/adr/0042-model-http-client-recovery-uses-generations.md @@ -0,0 +1,35 @@ +# 0042. Model HTTP client recovery uses leased generations + +Status: accepted 2026-08-31 + +## Context + +Issue #691 captured launch-scoped `TlsInitializationFailed` storms: once +`std.http.Client.request` failed during TLS construction, six retries and every +later TUI turn reused the same client and failed until process restart. The +existing #177 connection poison cannot help because no `Request` exists yet. +The launch client is also shared with concurrent root, compaction, title, +recap, and subagent traffic, so deinitializing it in place would race users. + +## Decision + +Model HTTP constructors lease an active launch-level client generation. A +request-construction `TlsInitializationFailed` retires that generation and +publishes a prewarmed replacement under one mutex. Existing requests keep the +retired generation alive through reference-counted leases; later retries and +turns resolve the original launch pointer to the replacement. Owned retired +generations are deinitialized only after their final lease releases. + +The original launch client is never reclaimed by the generation manager: +unrelated launch consumers still holding its pointer remain safe until normal +shutdown. All managed constructors wait for initial CA prewarm readiness, and +CA-prewarm plus request-construction failures leave distinct trace evidence. +Post-construction send/read failures retain #177's per-connection poison. + +## Consequences + +A recovered network can serve later root, synthetic, compaction, and child +model trajectories without replacing the process or durable session. Recovery +adds one mutex operation per model HTTP request and temporarily retains an old +client while requests from that generation are still in flight. WebSocket +transport remains independently managed; this record governs HTTP model calls. diff --git a/docs/adr/README.md b/docs/adr/README.md index e76f1c51..c2943a4e 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -52,6 +52,7 @@ record only when you need the evidence or the edge cases. | [0039](0039-local-tools-are-project-scripts.md) | Agent-authored local tools are project scripts under `.graff/tools/`; skills stay instructions. Runtime catalog extras, not `schema.effectiveRootSpecs`. | | [0040](0040-codedb-stays-when-licensed.md) | Ordinary reads use native `codedb` / `read_file`; codedb-pro is extra search, not the default reader. | | [0041](0041-tui-is-an-acp-client.md) | The fullscreen TUI is an in-process ACP client: session/prompt in, session/update thought/tool/text out. No child `graff acp`. | +| [0042](0042-model-http-client-recovery-uses-generations.md) | Model HTTP calls lease a recoverable client generation; request-construction TLS failure rotates safely without deinitializing in-flight users. | ## When to write one diff --git a/src/agent_request.zig b/src/agent_request.zig index 3cbf5ae0..8539cf90 100644 --- a/src/agent_request.zig +++ b/src/agent_request.zig @@ -113,9 +113,9 @@ pub const landing_note = "results beat dying mid-tool-call."; pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { - // Startup paints the prompt while CA loading continues. The root turn and - // title task rendezvous here, then issue their requests concurrently. + // Root and title requests rendezvous after launch-time CA loading. http.waitForClientReady(self.io); + if (http.takeCaWarmFailure()) if (self.tracer) |tr| tr.note("ca_prewarm_failed", "CA bundle rescan failed; request will use lazy TLS initialization"); if (self.registry) |reg| { if (@import("mcp_boot.zig").joinBeforeRequest(reg)) { self.invalidateRootTools(); @@ -346,6 +346,10 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { if (self.tracer) |tr| tr.api(self.label, self.sub, self.provider.model, 0, body.len, 0, 0, 0, true); return error.ApiError; } + if (err == error.TlsRequestConstructionFailed or err == error.TlsRequestConstructionCaWarmFailed) if (self.tracer) |tr| tr.note( + "tls_request_construction", + if (err == error.TlsRequestConstructionCaWarmFailed) "rotated shared HTTP client generation; replacement CA prewarm failed" else "rotated shared HTTP client generation", + ); if (attempt < max_attempts) { if (throttled) { // #retry-after: prefer the provider's Retry-After @@ -362,11 +366,8 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { if (self.tracer) |tr| tr.note("retry", what); self.sleepInterruptible(delay_ms) catch return error.Interrupted; } else { - // Transport flake (HttpConnectionClosing, a reset, - // a truncated TLS read): back off before a fresh - // connection. Rapid-fire retries against a - // just-closed keep-alive almost always re-fail - // (#86). 250ms·2ⁿ, capped at 4s over 6 tries; Esc cancels. + // Transport flakes back off; rapid retries against a + // just-closed keep-alive re-fail (#86). Cap: 4s/6 tries. const delay_ms = RetryPlan.delayMs(throttled, attempt); @import("turn_chrome.zig").emitRetryNotice(self.io, @errorName(err), attempt + 1, max_attempts); if (showRecoveredTransportRetry(self.call_kind)) diff --git a/src/agent_stream.zig b/src/agent_stream.zig index 7335450c..03533704 100644 --- a/src/agent_stream.zig +++ b/src/agent_stream.zig @@ -24,6 +24,7 @@ const reasoningDelta = @import("title.zig").reasoningDelta; const stream_tests = @import("agent_stream_tests.zig"); const http = @import("http.zig"); +const http_client = @import("http_client.zig"); const http_headers = @import("http_headers.zig"); const providerUserAgent = http.providerUserAgent; const capture5xxBodyStream = http.capture5xxBodyStream; @@ -52,6 +53,11 @@ pub fn postStream(self: *Agent, body: []const u8) ![]u8 { /// keep-alive cannot poison the WS→SSE handoff and every fallback retry dials /// from a clean pool. pub fn postStreamWithClient(self: *Agent, client: *std.http.Client, body: []const u8) ![]u8 { + http_client.waitForReady(client.io); + var lease = http_client.acquire(client); + defer lease.release(); + if (http_client.injectedConstructionTls(&lease)) |err| return err; + const transport = lease.client; const sink = engine_sink.forAgent(self); sink.emit(self.io, .stream_begin); // Every exit path — success, interrupt, transport error — tears down the @@ -109,14 +115,17 @@ pub fn postStreamWithClient(self: *Agent, client: *std.http.Client, body: []cons _ = drainSteerStdin(true); restoreStdin(o); }; - var req = try client.request(.POST, try std.Uri.parse(provider.url), .{ + var req = transport.request(.POST, try std.Uri.parse(provider.url), .{ .redirect_behavior = .unhandled, .headers = .{ .content_type = .{ .override = "application/json" }, .user_agent = providerUserAgent(provider), }, .extra_headers = extra, - }); + }) catch |err| { + if (err == error.TlsInitializationFailed) return http_client.constructionTlsError(transport); + return err; + }; defer req.deinit(); // A failed SEND leaves reader.state == .ready, which Request.deinit // reads as "connection still clean" and returns it to the keep-alive diff --git a/src/http.zig b/src/http.zig index 2aa9ac13..9794d27d 100644 --- a/src/http.zig +++ b/src/http.zig @@ -16,15 +16,9 @@ const Provider = provider_mod.Provider; const Agent = agent_mod.Agent; const headers = @import("http_headers.zig"); const stall = @import("http_stall.zig"); // #56: the watchdogs' pure budget arithmetic - -/// Launch-scoped gate installed while the shared client's CA bundle warms in -/// the background. Null in unit tests and standalone pre-client subcommands. -pub var g_client_ready: ?*Io.Event = null; - -pub fn waitForClientReady(io: Io) void { - if (g_client_ready) |ready| ready.waitUncancelable(io); -} - +const http_client = @import("http_client.zig"); +pub const waitForClientReady = http_client.waitForReady; +pub const takeCaWarmFailure = http_client.takeCaWarmFailure; pub const providerUserAgent = headers.userAgent; pub const providerHeaders = headers.providerHeaders; /// Test/call-site seam: the !live request path's POST, with an explicit conv id. @@ -113,15 +107,15 @@ test "retryAfterMs: seconds, ms preferred, cap, HTTP-date/none -> 0 (#retry-afte /// POST the request body; returns the raw response body (caller frees). /// Built on client.request, NOT client.fetch: fetch never exposes the -/// Request, so a failed body send could not be poisoned — std re-pooled the -/// dead connection (a failed SEND leaves reader.state == .ready, which -/// Request.deinit reads as "still clean") and findConnection handed the same -/// corpse to every retry and every later same-host request, so one -/// WriteFailed became a whole-session storm across compaction, [title], and -/// subagents (#177). Mirrors postStream's errdefer poison (agent_stream.zig). -/// The client and its connection pool stay shared across pool threads — -/// client.request is what fetch wraps and is equally thread-safe. +/// Request, so a failed body send could not be poisoned and std re-pooled the +/// dead connection across later retries, compaction, titles, and subagents +/// (#177). Mirrors postStream's errdefer poison (agent_stream.zig). fn post(gpa: Allocator, client: *std.http.Client, provider: Provider, body: []const u8, conv_id: ?[]const u8) ![]u8 { + http_client.waitForReady(client.io); + var lease = http_client.acquire(client); + defer lease.release(); + if (http_client.injectedConstructionTls(&lease)) |err| return err; + const transport = lease.client; var aw: Io.Writer.Allocating = .init(gpa); errdefer aw.deinit(); @@ -132,16 +126,19 @@ fn post(gpa: Allocator, client: *std.http.Client, provider: Provider, body: []co defer if (bearer.len > 0) gpa.free(bearer); var headers_buf: [12]std.http.Header = undefined; - const extra = headers.providerHeadersWithConv(client.io, provider, bearer, &headers_buf, conv_id); + const extra = headers.providerHeadersWithConv(transport.io, provider, bearer, &headers_buf, conv_id); - var req = try client.request(.POST, try std.Uri.parse(provider.url), .{ + var req = transport.request(.POST, try std.Uri.parse(provider.url), .{ .redirect_behavior = .unhandled, .headers = .{ .content_type = .{ .override = "application/json" }, .user_agent = providerUserAgent(provider), }, .extra_headers = extra, - }); + }) catch |err| { + if (err == error.TlsInitializationFailed) return http_client.constructionTlsError(transport); + return err; + }; defer req.deinit(); // The #177 poison: on ANY error make deinit discard this connection // instead of returning it to the keep-alive pool, so the retry (and diff --git a/src/http_client.zig b/src/http_client.zig new file mode 100644 index 00000000..016d7a38 --- /dev/null +++ b/src/http_client.zig @@ -0,0 +1,275 @@ +//! Launch-level HTTP client generations for model traffic. +//! +//! A `TlsInitializationFailed` raised by `Client.request` occurs before a +//! Request exists, so the connection-poison cleanup in http.zig cannot touch +//! it. Model calls lease the active generation through this module. On that +//! construction error, one caller rotates the generation; in-flight callers +//! keep their old lease, and owned retired clients are reclaimed only after +//! their final lease is released. + +const std = @import("std"); +const builtin = @import("builtin"); +const Io = std.Io; +const Allocator = std.mem.Allocator; +const warm = @import("http_warm.zig"); + +const Generation = struct { + client: *std.http.Client, + id: u64, + refs: usize = 0, + retired: bool = false, + owned: bool, + next: ?*Generation = null, +}; + +pub const RecoveryOutcome = enum { + unavailable, + rotated, + rotated_ca_warm_failed, + already_rotated, +}; + +pub const Recovery = struct { + gpa: Allocator, + io: Io, + mutex: Io.Mutex = .init, + original: Generation, + active: *Generation, + retired: ?*Generation = null, + next_id: u64 = 1, + warm_replacements: bool, + + pub fn init(self: *Recovery, gpa: Allocator, io: Io, original: *std.http.Client, warm_replacements: bool) void { + self.* = .{ + .gpa = gpa, + .io = io, + .original = .{ .client = original, .id = 0, .owned = false }, + .active = undefined, + .warm_replacements = warm_replacements, + }; + self.active = &self.original; + } + + pub fn deinit(self: *Recovery) void { + std.debug.assert(self.active.refs == 0); + if (self.active.owned) self.destroyOwned(self.active); + var cursor = self.retired; + while (cursor) |generation| { + const next = generation.next; + std.debug.assert(generation.refs == 0); + if (generation.owned) self.destroyOwned(generation); + cursor = next; + } + self.retired = null; + } + + pub fn acquire(self: *Recovery, requested: *std.http.Client) Lease { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + if (requested != self.original.client and requested != self.active.client) + return .{ .client = requested }; + self.active.refs += 1; + return .{ .client = self.active.client, .owner = self, .generation = self.active, .generation_id = self.active.id }; + } + + /// Retire the generation that failed while constructing a request. If a + /// concurrent caller already rotated it, this is a successful no-op: the + /// next retry will acquire that newer generation. + pub fn recoverConstructionTls(self: *Recovery, failed: *std.http.Client) RecoveryOutcome { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + if (failed != self.active.client) + return if (failed == self.original.client or self.isRetired(failed)) .already_rotated else .unavailable; + + const client = self.gpa.create(std.http.Client) catch return .unavailable; + client.* = .{ .allocator = self.gpa, .io = self.io }; + var ca_warm_failed = false; + if (self.warm_replacements) warm.prewarmCaBundle(client, self.gpa, self.io) catch { + ca_warm_failed = true; + }; + const generation = self.gpa.create(Generation) catch { + client.deinit(); + self.gpa.destroy(client); + return .unavailable; + }; + generation.* = .{ .client = client, .id = self.next_id, .owned = true }; + self.next_id += 1; + + const old = self.active; + old.retired = true; + old.next = self.retired; + self.retired = old; + self.active = generation; + if (old.refs == 0) self.reclaim(old); + return if (ca_warm_failed) .rotated_ca_warm_failed else .rotated; + } + + fn release(self: *Recovery, generation: *Generation) void { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + std.debug.assert(generation.refs > 0); + generation.refs -= 1; + if (generation.retired and generation.refs == 0) self.reclaim(generation); + } + + fn isRetired(self: *Recovery, client: *std.http.Client) bool { + var cursor = self.retired; + while (cursor) |generation| : (cursor = generation.next) { + if (generation.client == client) return true; + } + return false; + } + + fn reclaim(self: *Recovery, target: *Generation) void { + var link = &self.retired; + while (link.*) |generation| { + if (generation == target) { + link.* = generation.next; + generation.next = null; + generation.retired = false; + if (generation.owned) self.destroyOwned(generation); + return; + } + link = &generation.next; + } + } + + fn destroyOwned(self: *Recovery, generation: *Generation) void { + generation.client.deinit(); + self.gpa.destroy(generation.client); + self.gpa.destroy(generation); + } +}; + +pub const Lease = struct { + client: *std.http.Client, + owner: ?*Recovery = null, + generation: ?*Generation = null, + generation_id: u64 = 0, + + pub fn release(self: *Lease) void { + if (self.owner) |owner| owner.release(self.generation.?); + self.* = .{ .client = self.client }; + } +}; + +var g_recovery: ?*Recovery = null; +var g_client_ready: ?*Io.Event = null; +var g_ca_warm_failed: std.atomic.Value(bool) = .init(false); +const no_test_failure = std.math.maxInt(u64); +var g_test_fail_generation: std.atomic.Value(u64) = .init(no_test_failure); + +pub fn acquire(requested: *std.http.Client) Lease { + if (g_recovery) |recovery| return recovery.acquire(requested); + return .{ .client = requested }; +} + +pub fn constructionTlsError(failed: *std.http.Client) anyerror { + const outcome = if (g_recovery) |recovery| recovery.recoverConstructionTls(failed) else .unavailable; + return switch (outcome) { + .unavailable => error.TlsInitializationFailed, + .rotated_ca_warm_failed => error.TlsRequestConstructionCaWarmFailed, + .rotated, .already_rotated => error.TlsRequestConstructionFailed, + }; +} + +pub fn injectConstructionTlsForTest(generation: u64) void { + if (builtin.is_test) g_test_fail_generation.store(generation, .release); +} + +pub fn injectedConstructionTls(lease: *const Lease) ?anyerror { + if (!builtin.is_test) return null; + if (g_test_fail_generation.cmpxchgStrong(lease.generation_id, no_test_failure, .acq_rel, .acquire) == null) + return constructionTlsError(lease.client); + return null; +} + +pub fn waitForReady(io: Io) void { + if (g_client_ready) |ready| ready.waitUncancelable(io); +} + +pub fn takeCaWarmFailure() bool { + return g_ca_warm_failed.swap(false, .acq_rel); +} + +pub const Runtime = struct { + gpa: Allocator, + io: Io, + client: std.http.Client, + ready: Io.Event, + warm_future: Io.Future(void), + recovery: Recovery, + + pub fn init(self: *Runtime, gpa: Allocator, io: Io) void { + self.gpa = gpa; + self.io = io; + self.client = .{ .allocator = gpa, .io = io }; + self.ready = .unset; + self.recovery.init(gpa, io, &self.client, true); + g_recovery = &self.recovery; + g_client_ready = &self.ready; + g_ca_warm_failed.store(false, .release); + self.warm_future = io.async(warm.prewarmCaBundleTask, .{ &self.client, gpa, io, &self.ready, &g_ca_warm_failed }); + } + + pub fn deinit(self: *Runtime, await_io: Io) void { + _ = self.warm_future.await(await_io); + g_client_ready = null; + g_recovery = null; + self.recovery.deinit(); + self.client.deinit(); + } +}; + +test "request-construction TLS recovery reaches later root and child trajectories" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var failed_root = recovery.acquire(&original); + try std.testing.expectEqual(@as(u64, 0), failed_root.generation_id); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(failed_root.client)); + + var later_root = recovery.acquire(&original); + defer later_root.release(); + var child = recovery.acquire(&original); + defer child.release(); + try std.testing.expectEqual(@as(u64, 1), later_root.generation_id); + try std.testing.expectEqual(later_root.generation_id, child.generation_id); + try std.testing.expect(later_root.client == child.client); + try std.testing.expect(later_root.client != failed_root.client); + failed_root.release(); +} + +fn recoverTask(recovery: *Recovery, client: *std.http.Client) RecoveryOutcome { + return recovery.recoverConstructionTls(client); +} + +test "concurrent stale TLS reports rotate a generation only once" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var first = recovery.acquire(&original); + var concurrent = recovery.acquire(&original); + var first_fut = io.async(recoverTask, .{ &recovery, first.client }); + var second_fut = io.async(recoverTask, .{ &recovery, concurrent.client }); + const first_result = first_fut.await(io); + const second_result = second_fut.await(io); + try std.testing.expect(first_result != .unavailable); + try std.testing.expect(second_result != .unavailable); + try std.testing.expect(first_result != second_result); + var after = recovery.acquire(&original); + defer after.release(); + try std.testing.expectEqual(@as(u64, 1), after.generation_id); + first.release(); + concurrent.release(); +} diff --git a/src/http_client_integration_tests.zig b/src/http_client_integration_tests.zig new file mode 100644 index 00000000..7b5f92a7 --- /dev/null +++ b/src/http_client_integration_tests.zig @@ -0,0 +1,67 @@ +//! End-to-end model-POST regression for request-construction TLS recovery. + +const std = @import("std"); +const Io = std.Io; +const http = @import("http.zig"); +const http_client = @import("http_client.zig"); +const Provider = @import("provider.zig").Provider; + +fn serveOk(io: Io, server: *std.Io.net.Server) void { + for (0..2) |_| { + const conn = server.accept(io) catch return; + defer conn.close(io); + var read_buf: [4096]u8 = undefined; + var reader = std.Io.net.Stream.Reader.init(conn, io, &read_buf); + while (true) { + const line = (reader.interface.takeDelimiter('\n') catch return) orelse return; + if (line.len == 0 or (line.len == 1 and line[0] == '\r')) break; + } + _ = reader.interface.take(2) catch return; + var write_buf: [256]u8 = undefined; + var writer = std.Io.net.Stream.Writer.init(conn, io, &write_buf); + writer.interface.writeAll("HTTP/1.1 200 OK\r\ncontent-length: 2\r\nconnection: close\r\n\r\nok") catch return; + writer.interface.flush() catch return; + } +} + +test "one TLS-broken generation recovers later ordinary and child model POSTs" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + var server_future = io.async(serveOk, .{ io, &server }); + defer server_future.await(io); + + const bound = server.socket.address; + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{bound.getPort()}); + const provider: Provider = .{ + .id = "test", + .kind = .openai, + .auth = .x_api_key, + .url = url, + .api_key = "test", + .model = "test", + .context = 0, + }; + + http_client.injectConstructionTlsForTest(0); + try std.testing.expectError( + error.TlsRequestConstructionFailed, + http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-root-failed"), + ); + + const ordinary = try http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-root-later"); + defer gpa.free(ordinary); + try std.testing.expectEqualStrings("ok", ordinary); + + const child = try http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-child-later"); + defer gpa.free(child); + try std.testing.expectEqualStrings("ok", child); +} diff --git a/src/http_warm.zig b/src/http_warm.zig index ff5c88cf..d95d744b 100644 --- a/src/http_warm.zig +++ b/src/http_warm.zig @@ -5,15 +5,15 @@ const Io = std.Io; /// Pre-load the shared HTTP client's CA bundle single-threaded so concurrent /// agents never race Zig's lazy first-connect rescan. -pub fn prewarmCaBundle(client: *std.http.Client, gpa: std.mem.Allocator, io: Io) void { +pub fn prewarmCaBundle(client: *std.http.Client, gpa: std.mem.Allocator, io: Io) !void { const now = Io.Clock.real.now(io); - client.ca_bundle.rescan(gpa, io, now) catch return; + try client.ca_bundle.rescan(gpa, io, now); client.now = now; } /// Warm off the launch critical path. Outbound users wait on /// `http.g_client_ready`, so prompt painting can overlap the scan safely. -pub fn prewarmCaBundleTask(client: *std.http.Client, gpa: std.mem.Allocator, io: Io, ready: *Io.Event) void { +pub fn prewarmCaBundleTask(client: *std.http.Client, gpa: std.mem.Allocator, io: Io, ready: *Io.Event, failed: *std.atomic.Value(bool)) void { defer ready.set(io); - prewarmCaBundle(client, gpa, io); + prewarmCaBundle(client, gpa, io) catch failed.store(true, .release); } diff --git a/src/main.zig b/src/main.zig index e6f5c174..48367233 100644 --- a/src/main.zig +++ b/src/main.zig @@ -4,9 +4,7 @@ const std = @import("std"); pub const panic = @import("tui").restore.Panic; // leave the alt screen BEFORE std prints a panic, or the restore sequence erases the trace (#535) const Io = std.Io; -const http_warm = @import("http_warm.zig"); -pub const prewarmCaBundle = http_warm.prewarmCaBundle; -const prewarmCaBundleTask = http_warm.prewarmCaBundleTask; +const http_client = @import("http_client.zig"); const Value = std.json.Value; const Allocator = std.mem.Allocator; const mcp = @import("mcp.zig"); @@ -292,16 +290,11 @@ pub fn main(init: std.process.Init) !void { const stale_saved_model = resolved_keys.stale_saved_model; const preferred_provider = resolved_keys.preferred_provider; const codex_account = resolved_keys.codex_account; - var client: std.http.Client = .{ .allocator = gpa, .io = io }; - defer client.deinit(); - var client_ready: Io.Event = .unset; - http.g_client_ready = &client_ready; - var client_warm_fut = io.async(prewarmCaBundleTask, .{ &client, gpa, io, &client_ready }); + var client_runtime: http_client.Runtime = undefined; + client_runtime.init(gpa, io); + defer client_runtime.deinit(startup_timing.shutdown_trace.at(io, "ca-warm-await")); + const client = &client_runtime.client; boot.mark(io, "CA warm scheduled"); - defer { - _ = client_warm_fut.await(startup_timing.shutdown_trace.at(io, "ca-warm-await")); - http.g_client_ready = null; - } var stdin_buf: [64 * 1024]u8 = undefined; var stdin_reader = Io.File.stdin().reader(io, &stdin_buf); const in = &stdin_reader.interface; @@ -309,7 +302,7 @@ pub fn main(init: std.process.Init) !void { var stdout_writer = Io.File.stdout().writer(io, &stdout_buf); const out = &stdout_writer.interface; g_out = out; - if (try session_start.runTitleCommand(io, gpa, arena, &client, default_provider, out, flags, &invocation_budget)) return; + if (try session_start.runTitleCommand(io, gpa, arena, client, default_provider, out, flags, &invocation_budget)) return; // Generate identity before opening either JSONL. The score channel and both // files share this run id; session_id is a separate runtime correlation id. session_start.initScoreRunId(io); @@ -359,7 +352,7 @@ pub fn main(init: std.process.Init) !void { } traj.node(.{ .kind = "session", .version = harness_version, .unix_ms = unixMs(io) }); - var telem = session_start.initTelemetry(io, gpa, &client, init.environ_map, flags, default_telemetry_endpoint); + var telem = session_start.initTelemetry(io, gpa, client, init.environ_map, flags, default_telemetry_endpoint); telemetry.g_telem = &telem; if (init.environ_map.get("GRAFF_FLEET")) |fv| { g_fleet = !(std.ascii.eqlIgnoreCase(fv, "off") or std.mem.eql(u8, fv, "0") or std.ascii.eqlIgnoreCase(fv, "false") or std.ascii.eqlIgnoreCase(fv, "no")); @@ -425,7 +418,7 @@ pub fn main(init: std.process.Init) !void { // Root Agent construction + post-construction config (session name, persisted thinking/goal/eval settings, session-start trace note) + the // backgrounded fleet-champion pull live in session_start.zig. `root`'s pointer fields (snapshots/client/tracer/approvals/registry) all reference // already-stable main()-owned storage passed in by address, so returning the constructed Agent by value here is safe. - var root = try session_run.buildRootAgent(gpa, arena, io, &client, default_provider, subagent_provider, init.environ_map, out, in, registry, &approvals, &tracer, sys_normal, &snaps, flags, telem.endpoint); + var root = try session_run.buildRootAgent(gpa, arena, io, client, default_provider, subagent_provider, init.environ_map, out, in, registry, &approvals, &tracer, sys_normal, &snaps, flags, telem.endpoint); root.run_budget = &invocation_budget; root.model_catalog = resolved_keys.model_catalog; root.stored_keys_loaded = resolved_keys.stored_keys_loaded; @@ -439,7 +432,7 @@ pub fn main(init: std.process.Init) !void { // JSONL and the privacy-projected upload are independent sinks; the Boot // owns both, wired and torn down in LIFO order (behavior_trace.zig). var behavior_buf: [8 * 1024]u8 = undefined; - var behavior_boot = behavior_trace.boot(io, gpa, &client, init.environ_map, telem.endpoint, telem.auth_key, telem.install_id, telem.client_name, harness_version, &behavior_buf); + var behavior_boot = behavior_trace.boot(io, gpa, client, init.environ_map, telem.endpoint, telem.auth_key, telem.install_id, telem.client_name, harness_version, &behavior_buf); behavior_boot.link(&tracer); // A dead local sink must not be silent: the collision that disabled local // capture in every session shipped invisibly because every failure path @@ -484,7 +477,7 @@ pub fn main(init: std.process.Init) !void { // `graff` is the default session. TTY `graff repl` / `graff tui` open the Grok-style pager. // `graff acp` (acp.zig) is the same idea over Zed's stdio Agent Client Protocol. Both self-contained — each exits after. - if (try session_run.runReplCommand(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), &client, in, out, arena, flags) or try @import("acp.zig").runAcpCommand(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), &client, in, out, arena, flags) or try @import("tui_launch.zig").maybeRun(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), &client, arena, flags, json_mode, g_cwd_display)) return; + if (try session_run.runReplCommand(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), client, in, out, arena, flags) or try @import("acp.zig").runAcpCommand(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), client, in, out, arena, flags) or try @import("tui_launch.zig").maybeRun(gpa, io, init.environ_map, &root, @import("bench_priors.zig").noteKeys(&keys), client, arena, flags, json_mode, g_cwd_display)) return; // One-shot print mode: run the single prompt to completion, print the final text to stdout, exit. if (flags.oneshot_prompt) |prompt_text| { try session_run.runOneshotPrompt(gpa, io, arena, &root, @import("bench_priors.zig").noteKeys(&keys), &tracer, out, prompt_text); // one-shot exits before loop_ctx below — capture keys for sub-first routing here too @@ -584,6 +577,8 @@ test { // pull in tests from imported modules (mcp.zig) _ = @import("mcp.zig"); _ = @import("mcp_rpc.zig"); _ = @import("main_test.zig"); + _ = @import("http_client.zig"); + _ = @import("http_client_integration_tests.zig"); // A module whose tests must run needs an explicit reference here (a plain @import elsewhere compiles to nothing); scripts/eval-tier1.sh --only reach catches one. _ = @import("test_hooks.zig"); // unreached modules; their tests were silently skipped _ = @import("agent_overflow_tests.zig"); // #414: and, through it, agent_overflow.zig's table tests From f334bb5225e834141a259f19d519f309dd87fb31 Mon Sep 17 00:00:00 2001 From: yxlyx <85774423+yxlyx@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:05:21 +0800 Subject: [PATCH 2/2] fix(tui): harden TLS recovery lifecycle Close model-client admission before teardown, drain active generation leases and CA-ready waiters, and serialize global lookup with destruction. This prevents ReleaseFast use-after-free races while allowing unrelated HTTP clients to remain usable.\n\nCorrect retry accounting so the advertised limit is the actual total, preserve throttle/server failure attribution, and add deterministic real-TLS, concurrent, allocation, TUI-root, foreground-child, and background-child recovery coverage. Remove the unrequested ADR and raise the test-count ratchet to the verified suite size.\n\nCo-Authored-By: Codegraff --- ...l-http-client-recovery-uses-generations.md | 35 -- docs/adr/README.md | 1 - scripts/eval/tier1-manifest.json | 2 +- src/agent_request.zig | 15 +- src/agent_stream.zig | 1 + src/http.zig | 1 + src/http_client.zig | 245 ++++++-- src/http_client_integration_tests.zig | 549 ++++++++++++++++-- src/http_client_tests.zig | 294 ++++++++++ src/http_client_trajectory_tests.zig | 60 ++ src/main.zig | 2 + 11 files changed, 1063 insertions(+), 142 deletions(-) delete mode 100644 docs/adr/0042-model-http-client-recovery-uses-generations.md create mode 100644 src/http_client_tests.zig create mode 100644 src/http_client_trajectory_tests.zig diff --git a/docs/adr/0042-model-http-client-recovery-uses-generations.md b/docs/adr/0042-model-http-client-recovery-uses-generations.md deleted file mode 100644 index 08d19ae2..00000000 --- a/docs/adr/0042-model-http-client-recovery-uses-generations.md +++ /dev/null @@ -1,35 +0,0 @@ -# 0042. Model HTTP client recovery uses leased generations - -Status: accepted 2026-08-31 - -## Context - -Issue #691 captured launch-scoped `TlsInitializationFailed` storms: once -`std.http.Client.request` failed during TLS construction, six retries and every -later TUI turn reused the same client and failed until process restart. The -existing #177 connection poison cannot help because no `Request` exists yet. -The launch client is also shared with concurrent root, compaction, title, -recap, and subagent traffic, so deinitializing it in place would race users. - -## Decision - -Model HTTP constructors lease an active launch-level client generation. A -request-construction `TlsInitializationFailed` retires that generation and -publishes a prewarmed replacement under one mutex. Existing requests keep the -retired generation alive through reference-counted leases; later retries and -turns resolve the original launch pointer to the replacement. Owned retired -generations are deinitialized only after their final lease releases. - -The original launch client is never reclaimed by the generation manager: -unrelated launch consumers still holding its pointer remain safe until normal -shutdown. All managed constructors wait for initial CA prewarm readiness, and -CA-prewarm plus request-construction failures leave distinct trace evidence. -Post-construction send/read failures retain #177's per-connection poison. - -## Consequences - -A recovered network can serve later root, synthetic, compaction, and child -model trajectories without replacing the process or durable session. Recovery -adds one mutex operation per model HTTP request and temporarily retains an old -client while requests from that generation are still in flight. WebSocket -transport remains independently managed; this record governs HTTP model calls. diff --git a/docs/adr/README.md b/docs/adr/README.md index c2943a4e..e76f1c51 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -52,7 +52,6 @@ record only when you need the evidence or the edge cases. | [0039](0039-local-tools-are-project-scripts.md) | Agent-authored local tools are project scripts under `.graff/tools/`; skills stay instructions. Runtime catalog extras, not `schema.effectiveRootSpecs`. | | [0040](0040-codedb-stays-when-licensed.md) | Ordinary reads use native `codedb` / `read_file`; codedb-pro is extra search, not the default reader. | | [0041](0041-tui-is-an-acp-client.md) | The fullscreen TUI is an in-process ACP client: session/prompt in, session/update thought/tool/text out. No child `graff acp`. | -| [0042](0042-model-http-client-recovery-uses-generations.md) | Model HTTP calls lease a recoverable client generation; request-construction TLS failure rotates safely without deinitializing in-flight users. | ## When to write one diff --git a/scripts/eval/tier1-manifest.json b/scripts/eval/tier1-manifest.json index cd4c2181..b7953261 100644 --- a/scripts/eval/tier1-manifest.json +++ b/scripts/eval/tier1-manifest.json @@ -15,7 +15,7 @@ "src/repl.zig", "TUI/root.zig" ], - "test_count_baseline": 1745, + "test_count_baseline": 1783, "test_count_slack": 25, "required_invariants": [ { diff --git a/src/agent_request.zig b/src/agent_request.zig index 8539cf90..f5abc30a 100644 --- a/src/agent_request.zig +++ b/src/agent_request.zig @@ -244,6 +244,7 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { // error.ApiError so the REPL returns to the prompt, never crashes. const resp_body = blk: { var attempt: usize = 0; + var retry_limit: ?usize = null; while (true) : (attempt += 1) { var conv_buf: [96]u8 = undefined; const conv = http_headers.promptCacheKey(self.io, self.label, self, &conv_buf); @@ -327,7 +328,8 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { // (1s·2ⁿ, capped at 8s; Esc cancels) and allow a few // more attempts than a plain transport flake gets. const throttled = err == error.RateLimited or err == error.ServerError; - const max_attempts: usize = RetryPlan.maxAttempts(throttled); + const max_attempts = retry_limit orelse RetryPlan.maxAttempts(throttled); + retry_limit = max_attempts; // #opencode-parity: a 429 that's a billing/quota cap (not // transient throttling) won't clear by retrying — fail fast so // cross-provider /fallback can take over, instead of burning all @@ -350,7 +352,7 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { "tls_request_construction", if (err == error.TlsRequestConstructionCaWarmFailed) "rotated shared HTTP client generation; replacement CA prewarm failed" else "rotated shared HTTP client generation", ); - if (attempt < max_attempts) { + if (attempt + 1 < max_attempts) { if (throttled) { // #retry-after: prefer the provider's Retry-After // (429/503) over our computed backoff, capped — like @@ -381,12 +383,9 @@ pub fn request(self: *Agent, tools_in: ?[]const u8) !std.json.ObjectMap { continue; } try self.say("[request failed: {t} — giving up this turn]\n", .{err}); - // Network give-up is its own error kind: the ApiError - // handler's last_api_error would otherwise be an API - // envelope, stale or null on a pure transport failure — - // record the real reason so the failed turn's --json error - // event and trajectory node preserve it (#86). - self.last_api_error = std.fmt.allocPrint(self.arena, "network error: {s} (gave up after {d} attempts)", .{ @errorName(err), max_attempts }) catch null; + // Preserve whether this was provider throttling or a transport failure in the failed turn's JSON/trajectory (#86). + const failure_kind = if (err == error.RateLimited) "rate limited (429)" else if (err == error.ServerError) "server error (5xx)" else "network error"; + self.last_api_error = std.fmt.allocPrint(self.arena, "{s}: {s} (gave up after {d} attempts)", .{ failure_kind, @errorName(err), max_attempts }) catch null; self.last_request_write_failed = std.mem.eql(u8, @errorName(err), "WriteFailed"); if (telemetry.g_telem) |t| t.errorEvent("net", @errorName(err)); if (self.tracer) |tr| tr.api(self.label, self.sub, self.provider.model, 0, body.len, 0, 0, 0, true); diff --git a/src/agent_stream.zig b/src/agent_stream.zig index 03533704..142841b9 100644 --- a/src/agent_stream.zig +++ b/src/agent_stream.zig @@ -56,6 +56,7 @@ pub fn postStreamWithClient(self: *Agent, client: *std.http.Client, body: []cons http_client.waitForReady(client.io); var lease = http_client.acquire(client); defer lease.release(); + if (!lease.available) return error.Canceled; if (http_client.injectedConstructionTls(&lease)) |err| return err; const transport = lease.client; const sink = engine_sink.forAgent(self); diff --git a/src/http.zig b/src/http.zig index 9794d27d..fce77266 100644 --- a/src/http.zig +++ b/src/http.zig @@ -114,6 +114,7 @@ fn post(gpa: Allocator, client: *std.http.Client, provider: Provider, body: []co http_client.waitForReady(client.io); var lease = http_client.acquire(client); defer lease.release(); + if (!lease.available) return error.Canceled; if (http_client.injectedConstructionTls(&lease)) |err| return err; const transport = lease.client; var aw: Io.Writer.Allocating = .init(gpa); diff --git a/src/http_client.zig b/src/http_client.zig index 016d7a38..c476a5cd 100644 --- a/src/http_client.zig +++ b/src/http_client.zig @@ -29,14 +29,25 @@ pub const RecoveryOutcome = enum { already_rotated, }; +pub const Stats = struct { + active_id: u64, + active_refs: usize, + retired: usize, + total_refs: usize, + shutting_down: bool, +}; + pub const Recovery = struct { gpa: Allocator, io: Io, mutex: Io.Mutex = .init, + idle: Io.Condition = .init, original: Generation, active: *Generation, retired: ?*Generation = null, next_id: u64 = 1, + total_refs: usize = 0, + shutting_down: bool = false, warm_replacements: bool, pub fn init(self: *Recovery, gpa: Allocator, io: Io, original: *std.http.Client, warm_replacements: bool) void { @@ -50,7 +61,21 @@ pub const Recovery = struct { self.active = &self.original; } + pub fn beginShutdown(self: *Recovery) void { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + self.shutting_down = true; + } + + pub fn shutdown(self: *Recovery) void { + self.beginShutdown(); + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + while (self.total_refs != 0) self.idle.waitUncancelable(self.io, &self.mutex); + } + pub fn deinit(self: *Recovery) void { + self.shutdown(); std.debug.assert(self.active.refs == 0); if (self.active.owned) self.destroyOwned(self.active); var cursor = self.retired; @@ -68,25 +93,52 @@ pub const Recovery = struct { defer self.mutex.unlock(self.io); if (requested != self.original.client and requested != self.active.client) return .{ .client = requested }; + if (self.shutting_down) return .{ .client = requested, .available = false }; self.active.refs += 1; + self.total_refs += 1; return .{ .client = self.active.client, .owner = self, .generation = self.active, .generation_id = self.active.id }; } + pub fn stats(self: *Recovery) Stats { + self.mutex.lockUncancelable(self.io); + defer self.mutex.unlock(self.io); + var retired: usize = 0; + var cursor = self.retired; + while (cursor) |generation| : (cursor = generation.next) retired += 1; + return .{ + .active_id = self.active.id, + .active_refs = self.active.refs, + .retired = retired, + .total_refs = self.total_refs, + .shutting_down = self.shutting_down, + }; + } + /// Retire the generation that failed while constructing a request. If a /// concurrent caller already rotated it, this is a successful no-op: the /// next retry will acquire that newer generation. pub fn recoverConstructionTls(self: *Recovery, failed: *std.http.Client) RecoveryOutcome { self.mutex.lockUncancelable(self.io); defer self.mutex.unlock(self.io); + if (self.shutting_down) return .unavailable; if (failed != self.active.client) return if (failed == self.original.client or self.isRetired(failed)) .already_rotated else .unavailable; const client = self.gpa.create(std.http.Client) catch return .unavailable; client.* = .{ .allocator = self.gpa, .io = self.io }; var ca_warm_failed = false; - if (self.warm_replacements) warm.prewarmCaBundle(client, self.gpa, self.io) catch { - ca_warm_failed = true; - }; + if (self.warm_replacements) { + if (builtin.is_test and g_test_fail_ca_warm.swap(false, .acq_rel)) { + ca_warm_failed = true; + } else warm.prewarmCaBundle(client, self.gpa, self.io) catch { + ca_warm_failed = true; + }; + } + if (builtin.is_test and g_test_fail_generation_alloc.swap(false, .acq_rel)) { + client.deinit(); + self.gpa.destroy(client); + return .unavailable; + } const generation = self.gpa.create(Generation) catch { client.deinit(); self.gpa.destroy(client); @@ -109,7 +161,9 @@ pub const Recovery = struct { defer self.mutex.unlock(self.io); std.debug.assert(generation.refs > 0); generation.refs -= 1; + self.total_refs -= 1; if (generation.retired and generation.refs == 0) self.reclaim(generation); + if (self.shutting_down and self.total_refs == 0) self.idle.broadcast(self.io); } fn isRetired(self: *Recovery, client: *std.http.Client) bool { @@ -146,6 +200,7 @@ pub const Lease = struct { owner: ?*Recovery = null, generation: ?*Generation = null, generation_id: u64 = 0, + available: bool = true, pub fn release(self: *Lease) void { if (self.owner) |owner| owner.release(self.generation.?); @@ -153,18 +208,35 @@ pub const Lease = struct { } }; +var g_lifecycle_mutex: Io.Mutex = .init; +var g_lifecycle_idle: Io.Condition = .init; +var g_ready_waiters: usize = 0; +var g_closing: bool = false; var g_recovery: ?*Recovery = null; var g_client_ready: ?*Io.Event = null; +var g_closed_client: ?*std.http.Client = null; +var g_test_wait_entered: ?*Io.Event = null; var g_ca_warm_failed: std.atomic.Value(bool) = .init(false); const no_test_failure = std.math.maxInt(u64); var g_test_fail_generation: std.atomic.Value(u64) = .init(no_test_failure); +var g_test_fail_through_generation: std.atomic.Value(u64) = .init(no_test_failure); +var g_test_fail_ca_warm: std.atomic.Value(bool) = .init(false); +var g_test_fail_generation_alloc: std.atomic.Value(bool) = .init(false); +var g_test_tls_arrivals: ?*std.atomic.Value(usize) = null; +var g_test_tls_all_arrived: ?*Io.Event = null; +var g_test_tls_release: ?*Io.Event = null; pub fn acquire(requested: *std.http.Client) Lease { + g_lifecycle_mutex.lockUncancelable(requested.io); + defer g_lifecycle_mutex.unlock(requested.io); if (g_recovery) |recovery| return recovery.acquire(requested); + if (g_closed_client == requested) return .{ .client = requested, .available = false }; return .{ .client = requested }; } pub fn constructionTlsError(failed: *std.http.Client) anyerror { + g_lifecycle_mutex.lockUncancelable(failed.io); + defer g_lifecycle_mutex.unlock(failed.io); const outcome = if (g_recovery) |recovery| recovery.recoverConstructionTls(failed) else .unavailable; return switch (outcome) { .unavailable => error.TlsInitializationFailed, @@ -177,15 +249,99 @@ pub fn injectConstructionTlsForTest(generation: u64) void { if (builtin.is_test) g_test_fail_generation.store(generation, .release); } -pub fn injectedConstructionTls(lease: *const Lease) ?anyerror { - if (!builtin.is_test) return null; - if (g_test_fail_generation.cmpxchgStrong(lease.generation_id, no_test_failure, .acq_rel, .acquire) == null) - return constructionTlsError(lease.client); - return null; +pub fn injectConstructionTlsThroughGenerationForTest(last_generation: u64) void { + if (builtin.is_test) g_test_fail_through_generation.store(last_generation, .release); +} + +pub fn injectReplacementCaWarmFailureForTest() void { + if (builtin.is_test) g_test_fail_ca_warm.store(true, .release); +} + +pub fn injectGenerationAllocationFailureForTest() void { + if (builtin.is_test) g_test_fail_generation_alloc.store(true, .release); +} + +pub fn injectLaunchCaWarmFailureForTest() void { + if (builtin.is_test) g_ca_warm_failed.store(true, .release); +} + +pub fn installConstructionTlsBarrierForTest(arrivals: *std.atomic.Value(usize), all_arrived: *Io.Event, release: *Io.Event) void { + if (!builtin.is_test) return; + g_test_tls_arrivals = arrivals; + g_test_tls_all_arrived = all_arrived; + g_test_tls_release = release; +} + +inline fn resetTestHooks() void { + if (comptime !builtin.is_test) return; + g_test_wait_entered = null; + g_test_fail_generation.store(no_test_failure, .release); + g_test_fail_through_generation.store(no_test_failure, .release); + g_test_fail_ca_warm.store(false, .release); + g_test_fail_generation_alloc.store(false, .release); + g_test_tls_arrivals = null; + g_test_tls_all_arrived = null; + g_test_tls_release = null; +} + +pub fn installForTest(recovery: *Recovery, ready: ?*Io.Event, wait_entered: ?*Io.Event) void { + if (!builtin.is_test) return; + g_lifecycle_mutex.lockUncancelable(recovery.io); + defer g_lifecycle_mutex.unlock(recovery.io); + g_closing = false; + g_closed_client = null; + g_recovery = recovery; + g_client_ready = ready; + resetTestHooks(); + g_test_wait_entered = wait_entered; + g_ca_warm_failed.store(false, .release); +} + +pub fn uninstallForTest() void { + if (!builtin.is_test) return; + const recovery = g_recovery orelse return; + g_lifecycle_mutex.lockUncancelable(recovery.io); + defer g_lifecycle_mutex.unlock(recovery.io); + g_closing = true; + while (g_ready_waiters != 0) g_lifecycle_idle.waitUncancelable(recovery.io, &g_lifecycle_mutex); + g_recovery = null; + g_client_ready = null; + g_closed_client = null; + g_closing = false; + resetTestHooks(); +} + +pub inline fn injectedConstructionTls(lease: *const Lease) ?anyerror { + if (comptime !builtin.is_test) return null; + if (lease.owner == null) return null; + const through = g_test_fail_through_generation.load(.acquire); + const should_fail = through != no_test_failure and lease.generation_id <= through or + g_test_fail_generation.cmpxchgStrong(lease.generation_id, no_test_failure, .acq_rel, .acquire) == null; + if (!should_fail) return null; + if (g_test_tls_arrivals) |arrivals| { + if (arrivals.fetchAdd(1, .acq_rel) + 1 == 2) g_test_tls_all_arrived.?.set(lease.owner.?.io); + g_test_tls_release.?.waitUncancelable(lease.owner.?.io); + } + return constructionTlsError(lease.client); } pub fn waitForReady(io: Io) void { - if (g_client_ready) |ready| ready.waitUncancelable(io); + g_lifecycle_mutex.lockUncancelable(io); + if (g_closing or g_client_ready == null) { + g_lifecycle_mutex.unlock(io); + return; + } + const ready = g_client_ready.?; + g_ready_waiters += 1; + g_lifecycle_mutex.unlock(io); + + if (comptime builtin.is_test) if (g_test_wait_entered) |entered| entered.set(io); + ready.waitUncancelable(io); + + g_lifecycle_mutex.lockUncancelable(io); + g_ready_waiters -= 1; + if (g_closing and g_ready_waiters == 0) g_lifecycle_idle.broadcast(io); + g_lifecycle_mutex.unlock(io); } pub fn takeCaWarmFailure() bool { @@ -206,70 +362,35 @@ pub const Runtime = struct { self.client = .{ .allocator = gpa, .io = io }; self.ready = .unset; self.recovery.init(gpa, io, &self.client, true); + + g_lifecycle_mutex.lockUncancelable(io); + g_closing = false; + g_closed_client = null; g_recovery = &self.recovery; g_client_ready = &self.ready; + resetTestHooks(); g_ca_warm_failed.store(false, .release); + g_lifecycle_mutex.unlock(io); self.warm_future = io.async(warm.prewarmCaBundleTask, .{ &self.client, gpa, io, &self.ready, &g_ca_warm_failed }); } pub fn deinit(self: *Runtime, await_io: Io) void { + g_lifecycle_mutex.lockUncancelable(self.io); + g_closing = true; + self.recovery.beginShutdown(); + while (g_ready_waiters != 0) g_lifecycle_idle.waitUncancelable(self.io, &g_lifecycle_mutex); + g_lifecycle_mutex.unlock(self.io); + + self.recovery.shutdown(); _ = self.warm_future.await(await_io); + + g_lifecycle_mutex.lockUncancelable(self.io); g_client_ready = null; g_recovery = null; + g_closed_client = &self.client; + resetTestHooks(); self.recovery.deinit(); self.client.deinit(); + g_lifecycle_mutex.unlock(self.io); } }; - -test "request-construction TLS recovery reaches later root and child trajectories" { - const gpa = std.testing.allocator; - const io = std.testing.io; - var original: std.http.Client = .{ .allocator = gpa, .io = io }; - defer original.deinit(); - var recovery: Recovery = undefined; - recovery.init(gpa, io, &original, false); - defer recovery.deinit(); - - var failed_root = recovery.acquire(&original); - try std.testing.expectEqual(@as(u64, 0), failed_root.generation_id); - try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(failed_root.client)); - - var later_root = recovery.acquire(&original); - defer later_root.release(); - var child = recovery.acquire(&original); - defer child.release(); - try std.testing.expectEqual(@as(u64, 1), later_root.generation_id); - try std.testing.expectEqual(later_root.generation_id, child.generation_id); - try std.testing.expect(later_root.client == child.client); - try std.testing.expect(later_root.client != failed_root.client); - failed_root.release(); -} - -fn recoverTask(recovery: *Recovery, client: *std.http.Client) RecoveryOutcome { - return recovery.recoverConstructionTls(client); -} - -test "concurrent stale TLS reports rotate a generation only once" { - const gpa = std.testing.allocator; - const io = std.testing.io; - var original: std.http.Client = .{ .allocator = gpa, .io = io }; - defer original.deinit(); - var recovery: Recovery = undefined; - recovery.init(gpa, io, &original, false); - defer recovery.deinit(); - - var first = recovery.acquire(&original); - var concurrent = recovery.acquire(&original); - var first_fut = io.async(recoverTask, .{ &recovery, first.client }); - var second_fut = io.async(recoverTask, .{ &recovery, concurrent.client }); - const first_result = first_fut.await(io); - const second_result = second_fut.await(io); - try std.testing.expect(first_result != .unavailable); - try std.testing.expect(second_result != .unavailable); - try std.testing.expect(first_result != second_result); - var after = recovery.acquire(&original); - defer after.release(); - try std.testing.expectEqual(@as(u64, 1), after.generation_id); - first.release(); - concurrent.release(); -} diff --git a/src/http_client_integration_tests.zig b/src/http_client_integration_tests.zig index 7b5f92a7..74624e6d 100644 --- a/src/http_client_integration_tests.zig +++ b/src/http_client_integration_tests.zig @@ -1,30 +1,123 @@ -//! End-to-end model-POST regression for request-construction TLS recovery. +//! End-to-end model-transport regressions for request-construction TLS recovery. const std = @import("std"); const Io = std.Io; +const Agent = @import("agent.zig").Agent; const http = @import("http.zig"); const http_client = @import("http_client.zig"); +const mock = @import("agent_ws_mock.zig"); +const trace = @import("trace.zig"); const Provider = @import("provider.zig").Provider; +const Approvals = @import("approvals.zig").Approvals; +const repl_turn = @import("repl_turn.zig"); +const subagent_run = @import("subagent_run.zig"); +const tools = @import("tools.zig"); -fn serveOk(io: Io, server: *std.Io.net.Server) void { - for (0..2) |_| { +pub const Reply = struct { + status: []const u8 = "200 OK", + content_type: []const u8 = "application/json", + body: []const u8, +}; + +pub const chat_body = + \\{"choices":[{"index":0,"message":{"role":"assistant","content":"child-ok"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2}} +; +const sse_body = + "data: {\"type\":\"response.output_text.delta\",\"delta\":\"root-ok\"}\n\n" ++ + "data: {\"type\":\"response.completed\",\"response\":{\"id\":\"r1\",\"usage\":{\"input_tokens\":1,\"output_tokens\":1}}}\n\n"; +const chat_sse_body = + "data: {\"choices\":[{\"index\":0,\"delta\":{\"content\":\"root-ok\"},\"finish_reason\":null}]}\n\n" ++ + "data: {\"choices\":[{\"index\":0,\"delta\":{},\"finish_reason\":\"stop\"}]}\n\n" ++ + "data: [DONE]\n\n"; + +fn readRequest(reader: *std.Io.net.Stream.Reader) !void { + var content_length: usize = 0; + while (true) { + const line = (try reader.interface.takeDelimiter('\n')) orelse return error.EndOfStream; + if (line.len == 0 or (line.len == 1 and line[0] == '\r')) break; + if (std.ascii.startsWithIgnoreCase(line, "content-length:")) { + content_length = try std.fmt.parseInt(usize, std.mem.trim(u8, line[15..], " \t\r"), 10); + } + } + try reader.interface.discardAll(content_length); +} + +pub fn serveReplies(io: Io, server: *std.Io.net.Server, replies: []const Reply, accepted: *std.atomic.Value(usize)) void { + for (replies) |reply| { const conn = server.accept(io) catch return; - defer conn.close(io); - var read_buf: [4096]u8 = undefined; - var reader = std.Io.net.Stream.Reader.init(conn, io, &read_buf); - while (true) { - const line = (reader.interface.takeDelimiter('\n') catch return) orelse return; - if (line.len == 0 or (line.len == 1 and line[0] == '\r')) break; + { + defer conn.close(io); + _ = accepted.fetchAdd(1, .acq_rel); + var read_buf: [16 * 1024]u8 = undefined; + var reader = std.Io.net.Stream.Reader.init(conn, io, &read_buf); + readRequest(&reader) catch return; + var head_buf: [256]u8 = undefined; + const head = std.fmt.bufPrint( + &head_buf, + "HTTP/1.1 {s}\r\ncontent-type: {s}\r\ncontent-length: {d}\r\nconnection: close\r\n\r\n", + .{ reply.status, reply.content_type, reply.body.len }, + ) catch return; + var write_buf: [4096]u8 = undefined; + var writer = std.Io.net.Stream.Writer.init(conn, io, &write_buf); + writer.interface.writeAll(head) catch return; + writer.interface.writeAll(reply.body) catch return; + writer.interface.flush() catch return; } - _ = reader.interface.take(2) catch return; - var write_buf: [256]u8 = undefined; + } +} + +fn serveInvalidTls(io: Io, server: *std.Io.net.Server, count: usize, accepted: *std.atomic.Value(usize)) void { + for (0..count) |_| { + const conn = server.accept(io) catch return; + defer conn.close(io); + _ = accepted.fetchAdd(1, .acq_rel); + var write_buf: [128]u8 = undefined; var writer = std.Io.net.Stream.Writer.init(conn, io, &write_buf); - writer.interface.writeAll("HTTP/1.1 200 OK\r\ncontent-length: 2\r\nconnection: close\r\n\r\nok") catch return; - writer.interface.flush() catch return; + writer.interface.writeAll("HTTP/1.1 200 OK\r\ncontent-length: 0\r\n\r\n") catch continue; + writer.interface.flush() catch continue; } } -test "one TLS-broken generation recovers later ordinary and child model POSTs" { +pub fn releaseAccept(io: Io, server: *std.Io.net.Server) void { + const address = server.socket.address; + if (std.Io.net.IpAddress.connect(&address, io, .{ .mode = .stream })) |stream| stream.close(io) else |_| {} +} + +pub fn provider(url: []const u8) Provider { + return .{ + .id = "test", + .kind = .openai, + .auth = .x_api_key, + .url = url, + .api_key = "test", + .model = "test", + .context = 100_000, + }; +} + +fn childAgent(gpa: std.mem.Allocator, arena: std.mem.Allocator, io: Io, client: *std.http.Client, p: Provider) Agent { + return .{ + .gpa = gpa, + .arena = arena, + .io = io, + .client = client, + .provider = p, + .messages = std.json.Array.init(arena), + .sub = true, + .label = "test-child", + .out = null, + }; +} + +fn postTask(gpa: std.mem.Allocator, client: *std.http.Client, p: Provider) anyerror![]u8 { + return http.postWithConv(gpa, client, p, "{}", null); +} + +fn postWatchedTask(gpa: std.mem.Allocator, io: Io, client: *std.http.Client, p: Provider) anyerror![]u8 { + return http.postWatched(gpa, io, client, p, "{}", null); +} + +test "real malformed TLS handshakes traverse both production constructor catches" { const gpa = std.testing.allocator; const io = std.testing.io; var runtime: http_client.Runtime = undefined; @@ -35,33 +128,419 @@ test "one TLS-broken generation recovers later ordinary and child model POSTs" { var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); var server = try std.Io.net.IpAddress.listen(&address, io, .{}); defer server.deinit(io); - var server_future = io.async(serveOk, .{ io, &server }); + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveInvalidTls, .{ io, &server, 2, &accepted }); defer server_future.await(io); + defer releaseAccept(io, &server); - const bound = server.socket.address; var url_buf: [64]u8 = undefined; - const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{bound.getPort()}); - const provider: Provider = .{ - .id = "test", - .kind = .openai, - .auth = .x_api_key, - .url = url, - .api_key = "test", - .model = "test", - .context = 0, - }; - - http_client.injectConstructionTlsForTest(0); + const url = try std.fmt.bufPrint(&url_buf, "https://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); try std.testing.expectError( error.TlsRequestConstructionFailed, - http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-root-failed"), + http.postWithConv(gpa, &runtime.client, provider(url), "{}", null), ); + try std.testing.expectEqual(@as(u64, 1), runtime.recovery.stats().active_id); - const ordinary = try http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-root-later"); - defer gpa.free(ordinary); - try std.testing.expectEqualStrings("ok", ordinary); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var root = childAgent(gpa, arena_state.allocator(), io, &runtime.client, provider(url)); + root.sub = false; + try std.testing.expectError(error.TlsRequestConstructionFailed, root.postStreamWithClient(&runtime.client, "{}")); + try std.testing.expectEqual(@as(u64, 2), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); +} - const child = try http.postWithConv(gpa, &runtime.client, provider, "{}", "tui-child-later"); - defer gpa.free(child); - try std.testing.expectEqualStrings("ok", child); +test "TUI turn agent and actual runSub child share the recovered generation" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{ + .{ .content_type = "text/event-stream", .body = chat_sse_body }, + .{ .body = chat_body }, + }; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + const p = provider(url); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var output: Io.Writer.Allocating = .init(gpa); + defer output.deinit(); + var approvals: Approvals = .{ .yolo = true }; + var ctx = repl_turn.testCtx(&runtime.client); + ctx.provider = p; + var root = try repl_turn.turnAgent(&ctx, gpa, arena_state.allocator(), .{}, &output.writer, &approvals); + defer root.tools_used.deinit(gpa); + + http_client.injectConstructionTlsForTest(0); + _ = try root.request(null); + try std.testing.expect(std.mem.indexOf(u8, output.written(), "root-ok") != null); + try std.testing.expectEqual(@as(u64, 1), runtime.recovery.stats().active_id); + + const tool_ctx: tools.ToolCtx = .{ + .gpa = gpa, + .io = io, + .client = &runtime.client, + .provider = p, + .registry = null, + .from_sub = false, + .approvals = &approvals, + .tracer = null, + }; + const child = try subagent_run.runSub(tool_ctx, "subagent", "tls-test-child", "reply once", "test child", "", .shared_cwd, false, p, null); + defer gpa.free(child.output.text); + try std.testing.expect(!child.output.is_error); + try std.testing.expect(std.mem.indexOf(u8, child.output.text, "child-ok") != null); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); +} + +test "failed streaming root generation recovers later root and child requests" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{ + .{ .content_type = "text/event-stream", .body = sse_body }, + .{ .body = chat_body }, + }; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var root = mock.mockAgent(gpa, arena, io, url); + root.client = &runtime.client; + http_client.injectConstructionTlsForTest(0); + try std.testing.expectError(error.TlsRequestConstructionFailed, root.postStreamWithClient(root.client, "{}")); + + const streamed = try root.postStreamWithClient(root.client, "{}"); + defer gpa.free(streamed); + try std.testing.expect(std.mem.indexOf(u8, streamed, "response.completed") != null); + + var child = childAgent(gpa, arena, io, &runtime.client, provider(url)); + _ = try child.request(null); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); + const stats = runtime.recovery.stats(); + try std.testing.expectEqual(@as(u64, 1), stats.active_id); + try std.testing.expectEqual(@as(usize, 0), stats.active_refs); + try std.testing.expectEqual(@as(usize, 0), stats.retired); +} + +test "child retry ladder recovers within the same request" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{.{ .body = chat_body }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var child = childAgent(gpa, arena_state.allocator(), io, &runtime.client, provider(url)); + + http_client.injectConstructionTlsForTest(0); + _ = try child.request(null); + try std.testing.expectEqual(@as(usize, 1), accepted.load(.acquire)); + try std.testing.expectEqual(@as(u64, 1), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); +} + +test "later root and child recover after the retry ladder exhausts TLS generations" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{ + .{ .content_type = "text/event-stream", .body = chat_sse_body }, + .{ .body = chat_body }, + }; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + var output: Io.Writer.Allocating = .init(gpa); + defer output.deinit(); + var trace_output: Io.Writer.Allocating = .init(gpa); + defer trace_output.deinit(); + var tracer: trace.Tracer = .{ .io = io, .gpa = gpa, .out = &trace_output.writer, .start = Io.Timestamp.now(io, .awake) }; + + var root = childAgent(gpa, arena, io, &runtime.client, provider(url)); + root.sub = false; + root.label = "test-root"; + root.out = &output.writer; + root.tracer = &tracer; + http_client.injectConstructionTlsThroughGenerationForTest(5); + try std.testing.expectError(error.ApiError, root.request(null)); + try std.testing.expectEqual(@as(usize, 0), accepted.load(.acquire)); + try std.testing.expectEqual(@as(u64, 6), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 6), std.mem.count(u8, trace_output.written(), "\"ev\":\"tls_request_construction\"")); + try std.testing.expect(std.mem.indexOf(u8, root.last_api_error.?, "gave up after 6 attempts") != null); + + _ = try root.request(null); + try std.testing.expect(std.mem.indexOf(u8, output.written(), "root-ok") != null); + var child = childAgent(gpa, arena, io, &runtime.client, provider(url)); + const child_response = try child.request(null); + const choices = child_response.get("choices").?.array.items; + const content = choices[0].object.get("message").?.object.get("content").?.string; + try std.testing.expectEqualStrings("child-ok", content); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().retired); +} + +test "repeated TLS failures rotate multiple generations before recovery" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{.{ .body = "ok" }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + const p = provider(url); + http_client.injectConstructionTlsForTest(0); + try std.testing.expectError(error.TlsRequestConstructionFailed, http.postWithConv(gpa, &runtime.client, p, "{}", null)); + http_client.injectConstructionTlsForTest(1); + try std.testing.expectError(error.TlsRequestConstructionFailed, http.postWithConv(gpa, &runtime.client, p, "{}", null)); + const recovered = try http.postWithConv(gpa, &runtime.client, p, "{}", null); + defer gpa.free(recovered); + try std.testing.expectEqualStrings("ok", recovered); + try std.testing.expectEqual(@as(u64, 2), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().retired); +} + +test "simultaneous callers survive one shared generation failure" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{.{ .body = "ok" }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + const p = provider(url); + var arrivals: std.atomic.Value(usize) = .init(0); + var all_arrived: Io.Event = .unset; + var release: Io.Event = .unset; + http_client.installConstructionTlsBarrierForTest(&arrivals, &all_arrived, &release); + http_client.injectConstructionTlsThroughGenerationForTest(0); + var first = io.async(postTask, .{ gpa, &runtime.client, p }); + var second = io.async(postTask, .{ gpa, &runtime.client, p }); + all_arrived.waitUncancelable(io); + try std.testing.expectEqual(@as(usize, 2), runtime.recovery.stats().active_refs); + release.set(io); + const results = [_]anyerror![]u8{ first.await(io), second.await(io) }; + for (results) |result| { + if (result) |body| { + gpa.free(body); + return error.UnexpectedSuccess; + } else |err| try std.testing.expectEqual(error.TlsRequestConstructionFailed, err); + } + try std.testing.expectEqual(@as(u64, 1), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().retired); + + const recovered = try http.postWithConv(gpa, &runtime.client, p, "{}", null); + defer gpa.free(recovered); + try std.testing.expectEqualStrings("ok", recovered); + try std.testing.expectEqual(@as(usize, 1), accepted.load(.acquire)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); +} + +test "direct model POST waits for CA readiness before dialing" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: http_client.Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + var ready: Io.Event = .unset; + var wait_entered: Io.Event = .unset; + http_client.installForTest(&recovery, &ready, &wait_entered); + defer http_client.uninstallForTest(); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{.{ .body = "ok" }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/compact", .{server.socket.address.getPort()}); + var posted = io.async(postWatchedTask, .{ gpa, io, &original, provider(url) }); + wait_entered.waitUncancelable(io); + try std.testing.expectEqual(@as(usize, 0), accepted.load(.acquire)); + ready.set(io); + const body = try posted.await(io); + defer gpa.free(body); + try std.testing.expectEqualStrings("ok", body); + try std.testing.expectEqual(@as(usize, 1), accepted.load(.acquire)); +} + +test "launch CA failure trace is consumed once and does not block model requests" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{ .{ .body = chat_body }, .{ .body = chat_body } }; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var trace_output: Io.Writer.Allocating = .init(gpa); + defer trace_output.deinit(); + var tracer: trace.Tracer = .{ .io = io, .gpa = gpa, .out = &trace_output.writer, .start = Io.Timestamp.now(io, .awake) }; + var child = childAgent(gpa, arena_state.allocator(), io, &runtime.client, provider(url)); + child.tracer = &tracer; + + http_client.injectLaunchCaWarmFailureForTest(); + _ = try child.request(null); + _ = try child.request(null); + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, trace_output.written(), "\"ev\":\"ca_prewarm_failed\"")); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); +} + +test "Agent request traces replacement CA failure and recovers in the same retry ladder" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{ .{ .body = chat_body }, .{ .body = chat_body } }; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + var arena_state = std.heap.ArenaAllocator.init(gpa); + defer arena_state.deinit(); + var trace_output: Io.Writer.Allocating = .init(gpa); + defer trace_output.deinit(); + var tracer: trace.Tracer = .{ .io = io, .gpa = gpa, .out = &trace_output.writer, .start = Io.Timestamp.now(io, .awake) }; + var child = childAgent(gpa, arena_state.allocator(), io, &runtime.client, provider(url)); + child.tracer = &tracer; + + http_client.injectReplacementCaWarmFailureForTest(); + http_client.injectConstructionTlsForTest(0); + _ = try child.request(null); + try std.testing.expect(std.mem.indexOf( + u8, + trace_output.written(), + "rotated shared HTTP client generation; replacement CA prewarm failed", + ) != null); + _ = try child.request(null); + try std.testing.expectEqual(@as(usize, 2), accepted.load(.acquire)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); +} + +test "HTTP response error releases its generation lease" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http.waitForClientReady(io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]Reply{.{ .status = "500 Internal Server Error", .body = "upstream failed\n" }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(serveReplies, .{ io, &server, @as([]const Reply, &replies), &accepted }); + defer server_future.await(io); + defer releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + try std.testing.expectError(error.ServerError, http.postWithConv(gpa, &runtime.client, provider(url), "{}", null)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().retired); } diff --git a/src/http_client_tests.zig b/src/http_client_tests.zig new file mode 100644 index 00000000..2fd652a1 --- /dev/null +++ b/src/http_client_tests.zig @@ -0,0 +1,294 @@ +//! Unit tests for recoverable HTTP client generation lifetime and fault hooks. + +const std = @import("std"); +const http_client = @import("http_client.zig"); +const Recovery = http_client.Recovery; +const RecoveryOutcome = http_client.RecoveryOutcome; + +fn recoverTask(recovery: *Recovery, client: *std.http.Client) RecoveryOutcome { + return recovery.recoverConstructionTls(client); +} + +fn shutdownTask(recovery: *Recovery) void { + recovery.shutdown(); +} + +test "runtime teardown leaves the global transport closed to late acquisitions" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + runtime.deinit(io); + + var late = http_client.acquire(&runtime.client); + defer late.release(); + try std.testing.expect(!late.available); + + var unrelated: std.http.Client = .{ .allocator = gpa, .io = io }; + defer unrelated.deinit(); + var unmanaged = http_client.acquire(&unrelated); + defer unmanaged.release(); + try std.testing.expect(unmanaged.available); +} + +fn runtimeDeinitTask(runtime: *http_client.Runtime, io: std.Io) void { + runtime.deinit(io); +} + +test "runtime teardown closes admission before draining an active lease" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + http_client.waitForReady(io); + var lease = http_client.acquire(&runtime.client); + try std.testing.expect(lease.available); + + var teardown = io.async(runtimeDeinitTask, .{ &runtime, io }); + for (0..100) |_| { + if (runtime.recovery.stats().shutting_down) break; + try io.sleep(.fromMilliseconds(1), .awake); + } + try std.testing.expect(runtime.recovery.stats().shutting_down); + var denied = http_client.acquire(&runtime.client); + defer denied.release(); + try std.testing.expect(!denied.available); + + lease.release(); + _ = teardown.await(io); +} + +fn waitReadyTask(io: std.Io) void { + http_client.waitForReady(io); +} + +fn uninstallTask() void { + http_client.uninstallForTest(); +} + +test "lifecycle teardown drains callers already waiting for CA readiness" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + var ready: std.Io.Event = .unset; + var wait_entered: std.Io.Event = .unset; + http_client.installForTest(&recovery, &ready, &wait_entered); + + var waiter = io.async(waitReadyTask, .{io}); + wait_entered.waitUncancelable(io); + var teardown = io.async(uninstallTask, .{}); + ready.set(io); + _ = waiter.await(io); + _ = teardown.await(io); +} + +test "retired generation stays alive until its final concurrent lease releases" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var first = recovery.acquire(&original); + var second = recovery.acquire(&original); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(first.client)); + try std.testing.expectEqual(@as(usize, 1), recovery.stats().retired); + first.release(); + try std.testing.expectEqual(@as(usize, 1), recovery.stats().retired); + second.release(); + try std.testing.expectEqual(@as(usize, 0), recovery.stats().retired); +} + +test "replacement allocation failures keep the current generation usable" { + const backing = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = backing, .io = io }; + defer original.deinit(); + + for (0..2) |fail_index| { + var failing = std.testing.FailingAllocator.init(backing, .{ .fail_index = fail_index }); + var recovery: Recovery = undefined; + recovery.init(failing.allocator(), io, &original, false); + defer recovery.deinit(); + var lease = recovery.acquire(&original); + defer lease.release(); + try std.testing.expectEqual(RecoveryOutcome.unavailable, recovery.recoverConstructionTls(lease.client)); + try std.testing.expectEqual(@as(u64, 0), recovery.stats().active_id); + } +} + +test "unmanaged client TLS failure does not rotate the launch generation" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var unrelated: std.http.Client = .{ .allocator = gpa, .io = io }; + defer unrelated.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + try std.testing.expectEqual(RecoveryOutcome.unavailable, recovery.recoverConstructionTls(&unrelated)); + try std.testing.expectEqual(@as(u64, 0), recovery.stats().active_id); +} + +test "replacement CA warm failure is attributed to the triggering rotation" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, true); + defer recovery.deinit(); + + var failed = recovery.acquire(&original); + defer failed.release(); + http_client.injectReplacementCaWarmFailureForTest(); + try std.testing.expectEqual(RecoveryOutcome.rotated_ca_warm_failed, recovery.recoverConstructionTls(failed.client)); +} + +test "request-construction TLS recovery reaches later root and child trajectories" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var failed_root = recovery.acquire(&original); + try std.testing.expectEqual(@as(u64, 0), failed_root.generation_id); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(failed_root.client)); + + var later_root = recovery.acquire(&original); + defer later_root.release(); + var child = recovery.acquire(&original); + defer child.release(); + try std.testing.expectEqual(@as(u64, 1), later_root.generation_id); + try std.testing.expectEqual(later_root.generation_id, child.generation_id); + try std.testing.expect(later_root.client == child.client); + try std.testing.expect(later_root.client != failed_root.client); + failed_root.release(); +} + +test "concurrent stale TLS reports rotate a generation only once" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var first = recovery.acquire(&original); + var concurrent = recovery.acquire(&original); + var first_fut = io.async(recoverTask, .{ &recovery, first.client }); + var second_fut = io.async(recoverTask, .{ &recovery, concurrent.client }); + const first_result = first_fut.await(io); + const second_result = second_fut.await(io); + try std.testing.expect(first_result != .unavailable); + try std.testing.expect(second_result != .unavailable); + try std.testing.expect(first_result != second_result); + var after = recovery.acquire(&original); + defer after.release(); + try std.testing.expectEqual(@as(u64, 1), after.generation_id); + first.release(); + concurrent.release(); +} + +test "shutdown rejects new managed acquisitions and drains an in-flight replacement" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var initial = recovery.acquire(&original); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(initial.client)); + initial.release(); + var in_flight = recovery.acquire(&original); + try std.testing.expectEqual(@as(u64, 1), in_flight.generation_id); + + var shutdown = io.async(shutdownTask, .{&recovery}); + for (0..100) |_| { + if (recovery.stats().shutting_down) break; + try io.sleep(.fromMilliseconds(1), .awake); + } + try std.testing.expect(recovery.stats().shutting_down); + const denied = recovery.acquire(&original); + try std.testing.expect(!denied.available); + try std.testing.expectEqual(@as(usize, 1), recovery.stats().total_refs); + + in_flight.release(); + _ = shutdown.await(io); + try std.testing.expectEqual(@as(usize, 0), recovery.stats().total_refs); +} + +test "owned retired client remains alive until both stale leases release" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + + var initial = recovery.acquire(&original); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(initial.client)); + initial.release(); + var first = recovery.acquire(&original); + var second = recovery.acquire(&original); + try std.testing.expectEqual(RecoveryOutcome.rotated, recovery.recoverConstructionTls(first.client)); + try std.testing.expectEqual(@as(usize, 1), recovery.stats().retired); + first.release(); + try std.testing.expectEqual(@as(usize, 1), recovery.stats().retired); + second.release(); + try std.testing.expectEqual(@as(usize, 0), recovery.stats().retired); +} + +test "generation zero injection never intercepts an unmanaged client" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var unrelated: std.http.Client = .{ .allocator = gpa, .io = io }; + defer unrelated.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, false); + defer recovery.deinit(); + http_client.installForTest(&recovery, null, null); + defer http_client.uninstallForTest(); + + http_client.injectConstructionTlsForTest(0); + var unmanaged = recovery.acquire(&unrelated); + defer unmanaged.release(); + try std.testing.expect(http_client.injectedConstructionTls(&unmanaged) == null); + var managed = recovery.acquire(&original); + defer managed.release(); + try std.testing.expectEqual(error.TlsRequestConstructionFailed, http_client.injectedConstructionTls(&managed).?); +} + +test "post-prewarm generation allocation failure cleans up and preserves active client" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var original: std.http.Client = .{ .allocator = gpa, .io = io }; + defer original.deinit(); + var recovery: Recovery = undefined; + recovery.init(gpa, io, &original, true); + defer recovery.deinit(); + + var lease = recovery.acquire(&original); + defer lease.release(); + http_client.injectGenerationAllocationFailureForTest(); + try std.testing.expectEqual(RecoveryOutcome.unavailable, recovery.recoverConstructionTls(lease.client)); + try std.testing.expectEqual(@as(u64, 0), recovery.stats().active_id); + try std.testing.expect(lease.client == &original); +} diff --git a/src/http_client_trajectory_tests.zig b/src/http_client_trajectory_tests.zig new file mode 100644 index 00000000..5c25993c --- /dev/null +++ b/src/http_client_trajectory_tests.zig @@ -0,0 +1,60 @@ +//! Production-shaped model-call trajectories that sit above the transport adapter. + +const std = @import("std"); +const Io = std.Io; +const http_client = @import("http_client.zig"); +const support = @import("http_client_integration_tests.zig"); +const subagent = @import("subagent.zig"); +const tools = @import("tools.zig"); + +test "background subagent tool recovers TLS and completes through agent_output" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var runtime: http_client.Runtime = undefined; + runtime.init(gpa, io); + defer runtime.deinit(io); + http_client.waitForReady(io); + defer subagent.agentJobsReap(gpa, io); + + var address = try std.Io.net.IpAddress.parseLiteral("127.0.0.1:0"); + var server = try std.Io.net.IpAddress.listen(&address, io, .{}); + defer server.deinit(io); + const replies = [_]support.Reply{.{ .body = support.chat_body }}; + var accepted: std.atomic.Value(usize) = .init(0); + var server_future = io.async(support.serveReplies, .{ io, &server, @as([]const support.Reply, &replies), &accepted }); + defer server_future.await(io); + defer support.releaseAccept(io, &server); + + var url_buf: [64]u8 = undefined; + const url = try std.fmt.bufPrint(&url_buf, "http://127.0.0.1:{d}/v1/test", .{server.socket.address.getPort()}); + const ctx: tools.ToolCtx = .{ + .gpa = gpa, + .io = io, + .client = &runtime.client, + .provider = support.provider(url), + .registry = null, + .from_sub = false, + .approvals = null, + .tracer = null, + }; + const parsed = try std.json.parseFromSlice(std.json.Value, gpa, + \\{"description":"tls-background-child","prompt":"reply once","run_in_background":true} + , .{}); + defer parsed.deinit(); + + http_client.injectConstructionTlsForTest(0); + const spawned = try subagent.execSubagent(ctx, parsed.value); + defer gpa.free(spawned.text); + try std.testing.expect(!spawned.is_error); + const id_start = std.mem.indexOf(u8, spawned.text, "[agent ").? + "[agent ".len; + const id_end = std.mem.indexOfScalarPos(u8, spawned.text, id_start, ' ').?; + const id = try std.fmt.parseInt(u32, spawned.text[id_start..id_end], 10); + + const completed = try subagent.agentOutput(gpa, io, id, 1); + defer gpa.free(completed.text); + try std.testing.expect(!completed.is_error); + try std.testing.expect(std.mem.indexOf(u8, completed.text, "child-ok") != null); + try std.testing.expectEqual(@as(u64, 1), runtime.recovery.stats().active_id); + try std.testing.expectEqual(@as(usize, 1), accepted.load(.acquire)); + try std.testing.expectEqual(@as(usize, 0), runtime.recovery.stats().active_refs); +} diff --git a/src/main.zig b/src/main.zig index 48367233..17ec520c 100644 --- a/src/main.zig +++ b/src/main.zig @@ -578,7 +578,9 @@ test { // pull in tests from imported modules (mcp.zig) _ = @import("mcp_rpc.zig"); _ = @import("main_test.zig"); _ = @import("http_client.zig"); + _ = @import("http_client_tests.zig"); _ = @import("http_client_integration_tests.zig"); + _ = @import("http_client_trajectory_tests.zig"); // A module whose tests must run needs an explicit reference here (a plain @import elsewhere compiles to nothing); scripts/eval-tier1.sh --only reach catches one. _ = @import("test_hooks.zig"); // unreached modules; their tests were silently skipped _ = @import("agent_overflow_tests.zig"); // #414: and, through it, agent_overflow.zig's table tests