From fe0d2bf1a5006e03cd0e683f438926daf9f02005 Mon Sep 17 00:00:00 2001 From: Forketyfork Date: Sun, 13 Sep 2026 09:06:14 +0200 Subject: [PATCH] fix(mcp): initialize environment before handling requests Issue: Homebrew-installed architect-mcp aborted during tools/call because control-socket discovery accessed env before the MCP entrypoint initialized it. Solution: Initialize the process environment from std.process.Init before entering the stdio request loop and register the shared environment module for the MCP target. Add an end-to-end regression that invokes the real entrypoint with isolated runtime state and verifies the structured app-not-running response. --- build.zig | 1 + docs/ARCHITECTURE.md | 1 + docs/development.md | 5 +++ src/mcp/main.zig | 92 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+) diff --git a/build.zig b/build.zig index 4ed83fe6..5413c11f 100644 --- a/build.zig +++ b/build.zig @@ -65,6 +65,7 @@ pub fn build(b: *std.Build) void { control_mod.addImport("../posix_util.zig", posix_util_mod); control_mod.addImport("../wake_pipe.zig", wake_pipe_mod); mcp_mod.addImport("control", control_mod); + mcp_mod.addImport("../env.zig", env_mod); const assets_mod = b.createModule(.{ .root_source_file = b.path("assets/terminfo.zig"), .target = target, diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 09124407..f7ec2142 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -92,6 +92,7 @@ Platform Session Rendering UI Overlay - Session, Rendering, and UI Overlay layers never import from each other directly. All cross-layer communication flows through the Application layer or shared types. - UI components communicate with the application exclusively via the `UiAction` queue (never direct state mutation). - `main(init: std.process.Init)` passes `init.gpa` and `init.io` to `runtime.run(allocator, io, ...)`, which threads them through the application, session, and UI layers. I/O-owning structs store them together; worker contexts copy `io` when their thread outlives the spawner. +- The `architect-mcp` entrypoint initializes the shared process environment from `init.minimal.environ` before handling stdio requests, because control-socket discovery reads environment variables through `env.zig`. - Background threads are intentionally limited to five cases: the notification socket listener (`session/notify.zig`), the local control socket listener (`app/control.zig`), the PTY reader (`session/pty_reader.zig`), the bounded URL opener worker set (`os/open.zig`), and a quit-time agent-teardown worker in `app/runtime.zig`. The URL opener joins its active workers during runtime shutdown. The notification listener and control listener block in `poll(2)` on the listening socket plus a `WakePipe` self-pipe (`src/wake_pipe.zig`); shutdown stores the stop flag and signals the pipe, so no thread ever sleeps in a fixed-interval loop. The notification listener, control listener, and PTY reader also post a custom SDL wake event after queueing work or draining PTY bytes, so the frame loop breaks out of `SDL_WaitEventTimeout(...)` promptly during both idle and active-frame pacing. The PTY reader blocks in `poll(2)` on the master fds of all spawned sessions plus its `WakePipe`, which `register`/`retire` and shutdown signal; when one becomes readable, it drains it into that session's mutex-guarded ring buffer (`PtyOutputBuffer`, 1 MiB) — so producer processes are never backpressured by render pacing, and DEC-2026 sync windows close in the buffer as fast as the producer writes them. Sessions register their fd+buffer on spawn and retire it during teardown; reads happen only under the registry mutex, so `retire()` returning guarantees the reader can no longer touch the fd or buffer. The main thread's `processOutput` consumes from the buffer (VT parsing stays main-thread-only) and clears a shared `wake_pending` flag at the top of each frame; the reader posts at most one SDL wake event per frame via that flag. - Shutdown order is UI-first for teardown dependencies: `UiRoot.deinit()` runs before session teardown so components that reference sessions are released while session memory is still valid. - Runtime uses a one-shot teardown guard around UI cleanup so mixed `errdefer`/`defer` error unwind paths cannot deinitialize `UiRoot` twice. diff --git a/docs/development.md b/docs/development.md index 4b16d53c..d511e04c 100644 --- a/docs/development.md +++ b/docs/development.md @@ -101,6 +101,11 @@ its tests compile but silently never run. `scripts/check-test-registry.sh` (part of `just lint`) fails the build when a file with tests is missing from that block. +The MCP test binary drives a complete stdio `tools/call` request against an +isolated runtime directory and verifies the structured error returned when +Architect is not running. This also covers environment initialization before +control-socket discovery. + Check formatting and script linting: ```bash just lint diff --git a/src/mcp/main.zig b/src/mcp/main.zig index 4df2b0bc..b6534e61 100644 --- a/src/mcp/main.zig +++ b/src/mcp/main.zig @@ -1,5 +1,6 @@ const std = @import("std"); const control = @import("control"); +const env = @import("../env.zig"); const log = std.log.scoped(.mcp); const protocol_version = "2025-11-25"; @@ -16,6 +17,7 @@ const JsonRpcErrorCode = enum(i32) { }; pub fn main(init: std.process.Init) !void { + env.init(init.minimal.environ); try run(init.gpa, init.io, std.Io.File.stdin(), std.Io.File.stdout()); } @@ -523,6 +525,96 @@ test "tool failure response is an MCP tool error result" { try std.testing.expectEqualStrings("invalid_cwd", code.string); } +test "tools/call returns an app-not-running result without aborting" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + const runtime_dir_name = try std.fmt.allocPrint(allocator, ".tmp/mcp_runtime_{d}", .{std.c.getpid()}); + defer allocator.free(runtime_dir_name); + std.Io.Dir.cwd().createDirPath(io, runtime_dir_name) catch |err| switch (err) { + error.PathAlreadyExists => {}, + else => return err, + }; + defer std.Io.Dir.cwd().deleteDir(io, runtime_dir_name) catch |err| { + std.debug.print("cleanup failed: {}\n", .{err}); + }; + + const runtime_dir = try std.Io.Dir.cwd().realPathFileAlloc(io, runtime_dir_name, allocator); + defer allocator.free(runtime_dir); + + var environ_map = std.process.Environ.Map.init(allocator); + defer environ_map.deinit(); + try environ_map.put("XDG_RUNTIME_DIR", runtime_dir); + const environ_block = try environ_map.createPosixBlock(allocator, .{}); + defer environ_block.deinit(allocator); + const environ: std.process.Environ = .{ .block = environ_block }; + + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + const empty_args: []const [*:0]const u8 = &.{}; + const init: std.process.Init = .{ + .minimal = .{ + .environ = environ, + .args = .{ .vector = empty_args }, + }, + .arena = &arena, + .gpa = allocator, + .io = io, + .environ_map = &environ_map, + .preopens = .empty, + }; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + + { + const input = try tmp.dir.createFile(io, "input.jsonl", .{}); + defer input.close(io); + try input.writeStreamingAll( + io, + "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":{\"name\":\"spawn_session\",\"arguments\":{\"cwd\":\"/tmp\"}}}\n", + ); + } + + const input = try tmp.dir.openFile(io, "input.jsonl", .{}); + defer input.close(io); + const output = try tmp.dir.createFile(io, "output.jsonl", .{}); + defer output.close(io); + + const saved_stdin = std.c.dup(0); + if (saved_stdin < 0) return error.Unexpected; + const saved_stdout = std.c.dup(1); + if (saved_stdout < 0) { + _ = std.c.close(saved_stdin); + return error.Unexpected; + } + defer { + if (std.c.dup2(saved_stdin, 0) < 0) std.debug.print("failed to restore stdin\n", .{}); + if (std.c.dup2(saved_stdout, 1) < 0) std.debug.print("failed to restore stdout\n", .{}); + _ = std.c.close(saved_stdin); + _ = std.c.close(saved_stdout); + } + + if (std.c.dup2(input.handle, 0) < 0) return error.Unexpected; + if (std.c.dup2(output.handle, 1) < 0) return error.Unexpected; + try main(init); + if (std.c.dup2(saved_stdin, 0) < 0) return error.Unexpected; + if (std.c.dup2(saved_stdout, 1) < 0) return error.Unexpected; + + const response = try tmp.dir.readFileAlloc(io, "output.jsonl", allocator, .limited(16 * 1024)); + defer allocator.free(response); + var parsed = try std.json.parseFromSlice(std.json.Value, allocator, std.mem.trim(u8, response, "\n"), .{}); + defer parsed.deinit(); + + const result_value = parsed.value.object.get("result") orelse return error.TestUnexpectedResult; + const result = result_value.object; + const is_error = result.get("isError") orelse return error.TestUnexpectedResult; + try std.testing.expect(is_error.bool); + const structured_content = result.get("structuredContent") orelse return error.TestUnexpectedResult; + const code = structured_content.object.get("code") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("app_not_running", code.string); +} + test "run discards the rest of an oversized line" { const allocator = std.testing.allocator; const io = std.testing.io;