Replace tiny-process-library with SDK process code (CreateProcessW)#32
Merged
Replace tiny-process-library with SDK process code (CreateProcessW)#32
Conversation
Generated Python CLI scripts now look for the fastmcpp binary next to the script itself before falling back to PATH lookup. Fixes FileNotFoundError on Windows where the binary isn't in PATH.
218be36 to
211c615
Compare
…ssW) Replace the FetchContent dependency on tiny-process-library with internal cross-platform process code adapted from copilot-sdk-cpp. This eliminates an external dependency that was transitively inherited by downstream projects (libagents -> fastmcpp_core). Win32 implementation upgraded from CreateProcessA to CreateProcessW: - UTF-8/UTF-16 conversion via utf8_to_wide() helper - STARTUPINFOEXW with explicit handle inheritance (PROC_THREAD_ATTRIBUTE_HANDLE_LIST) - Job Object for automatic child cleanup - CREATE_NO_WINDOW to prevent console popups - Stderr redirected to NUL when not captured StdioTransport rewritten from callback-based (deque + condition_variable) to synchronous pipe model (direct read_line on stdout pipe), with a background stderr reader thread in keep-alive mode to prevent pipe buffer deadlock. Process liveness is checked during timeout polling (200ms intervals) for fast failure detection when servers crash. Also fixes two CI issues from PR #31: - openapi_provider.hpp: brace-init default param -> explicit Options{} - transports.hpp: add missing override on session_id/has_session New tests: stdio_lifecycle, stdio_stderr, stdio_timeout. All 89 tests pass.
211c615 to
8bf2fff
Compare
The template provider uses weakly_canonical() to resolve resource paths, but skill_path_ was stored as absolute().lexically_normal(). On macOS, /tmp is a symlink to /private/tmp, so weakly_canonical resolves it but lexically_normal does not -- causing the is_within prefix check to fail with "Skill path escapes root". Same issue on Windows with 8.3 short names. Using weakly_canonical for skill_path_ ensures both sides of the comparison use the same canonical form.
Test that SkillProvider and SkillsDirectoryProvider correctly handle template resource reads when the skill path goes through a symlink (POSIX) or NTFS junction (Windows). This catches the bug where skill_path_ stored as lexically_normal() diverged from weakly_canonical() in the template provider's is_within() check. Tests: - [link] Template reads through symlink/junction path - [link-dir] Directory provider through symlink/junction - [canonical-temp] Temp path with canonical differences - [escape] Path traversal rejection (../secret.txt) - [resources-mode] Resources mode through non-canonical path Key implementation note: uses require() instead of assert() to avoid side-effect-in-assert bugs in Release builds (NDEBUG strips assert).
5b90817 to
333c067
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tiny-process-librarywith internal cross-platform process code adapted from copilot-sdk-cppCreateProcessAtoCreateProcessW(Unicode) withSTARTUPINFOEXW, explicit handle inheritance, and Job Object child cleanupStdioTransportfrom callback-based (deque + condition_variable) to synchronous pipe model with background stderr readerstdio_lifecycle,stdio_stderr,stdio_timeoutMotivation
Downstream projects pulling
libagents -> fastmcpp_coretransitively inherit thetiny-process-librarydep even though all the needed process functionality already exists in the SDK code. This PR eliminates that extra dependency and unifies on one process abstraction.Changes
New files (
src/internal/):process.hpp— Cross-platform process API (fastmcpp::process::namespace)process_win32.cpp— Win32: CreateProcessW, STARTUPINFOEXW, explicit handle list, Job Object, CREATE_NO_WINDOWprocess_posix.cpp— POSIX: fork/exec with error pipe for exec failure detectionprocess.cpp— Platform dispatcherModified files:
CMakeLists.txt— Remove TinyProcessLib FetchContent block, addsrc/internal/process.cppsrc/client/transports.cpp— Rewrite StdioTransport internals (public API unchanged via pimpl)include/fastmcpp/client/transports.hpp— Add missingoverrideonsession_id()/has_session()(CI fix)include/fastmcpp/providers/openapi_provider.hpp— Fix brace-init default param for AppleClang (CI fix)tests/transports/stdio_failure.cpp— Remove#ifdef TINY_PROCESS_LIB_AVAILABLEguardTest plan
ctest --test-dir build -C Release)stdio_client— keep-alive mode, one-shot mode, counter state persistencestdio_failure— non-existent command throwsTransportErrorstdio_lifecycle— server crash, destructor cleanup, rapid sequential requestsstdio_stderr— log_file, log_stream, stderr in error messagesstdio_timeout— unresponsive server triggers 30s timeoutTINY_PROCESS_LIB/tiny_processremain in tree