fix(windows): unswallow installer failures, quote CreateProcessW args, document MotW#813
Open
DeusData wants to merge 2 commits into
Open
fix(windows): unswallow installer failures, quote CreateProcessW args, document MotW#813DeusData wants to merge 2 commits into
DeusData wants to merge 2 commits into
Conversation
…, document MotW Distilled from PR #702 (fixes #697): - install.ps1: replace the try/catch that swallowed `codebase-memory-mcp install` failures with a $LASTEXITCODE check that reports the exit code and fails the installer, instead of silently leaving no coding agent configured. - README: document Unblock-File for the Mark-of-the-Web restriction that blocks the downloaded install.ps1, plus the execution-policy escape hatch. - cbm_exec_no_shell (Windows): switch from _spawnvp, whose CRT does not quote arguments containing spaces (the taskkill filter "IMAGENAME eq codebase-memory-mcp.exe" arrived as three bare tokens), to CreateProcessW over a two-pass MSVC-convention quoted command line (cbm_build_cmdline, exposed for tests via compat_fs_internal.h). - UTF-8-correct widening: the quoted command line is assembled in UTF-8 bytes and converted once via cbm_utf8_to_wide, so non-ASCII arguments (e.g. a non-ASCII %USERPROFILE%) survive intact instead of being byte-widened as Latin-1 mojibake. - tests: regression guard for the #697 taskkill filter, MSVC quoting edge cases (empty arg, embedded quote, trailing backslashes), UTF-8 widening guards (2-byte sequence with explicit code points; mixed 2-/3-byte round-trip via cbm_utf8_to_wide), and live CreateProcessW spawn tests. All Windows-only, exercised by the Windows CI leg. Co-authored-by: ShauryaaSharma <shauryasofficial27@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.
Closes #697.
Distilled from #702 (thanks @ShauryaaSharma — credited via
Co-authored-by).What
Three Windows installer failures from #697, plus UTF-8-correct argument widening:
install.ps1— README now documentsUnblock-File .\install.ps1in both install paths, plus theSet-ExecutionPolicy -Scope Process Bypassescape hatch.install.ps1wrappedcodebase-memory-mcp install -yin atry/catchthat caught nothing useful (native commands don't throw) and printed "non-fatal". It now checks$LASTEXITCODE, reports the exit code, and fails the installer so users aren't left with a binary but no configured agent.taskkill /FI "IMAGENAME eq codebase-memory-mcp.exe"argument splitting —cbm_exec_no_shellused_spawnvp, whose CRT concatenates argv without quoting, so the filter arrived as three bare tokens (ERROR: Invalid argument/option - 'eq'). It now builds an MSVC-convention quoted command line (cbm_build_cmdline, two-pass sizing + write, exposed for tests viacompat_fs_internal.h) and spawns viaCreateProcessW.cbm_utf8_to_wide, so non-ASCII arguments (e.g. paths under a non-ASCII%USERPROFILE%) survive intact instead of being byte-widened as Latin-1 mojibake.Tests
Windows-only additions to the
securitysuite (exercised by the Windows CI leg):cmdline_taskkill_filter_is_single_quoted_token— the exact Windows: install.ps1 reports success but leaves MCP server unregistered (MotW + swallowed config errors + malformed taskkill) #697 regression.""), embedded quote, trailing backslashes before the closing quote, empty argv.cmdline_utf8_arg_is_widened_not_latin1— UTF-8C3 A9must collapse to U+00E9, not survive as two Latin-1 code points.cmdline_utf8_multibyte_roundtrips_via_utf8_to_wide— mixed 2-byte umlaut + 3-byte CJK argument round-trips identically tocbm_utf8_to_wideof the same fully-quoted UTF-8 command line; fails on Latin-1 byte-widening.CreateProcessWspawn tests (cmd /c exit 0/42, NULL argv).Verification
make -f Makefile.cbm cbm(-Werrorclean),lint-cigreen,securitysuite 34/34 (Windows-only tests compiled out; POSIX exec path unaffected — all C changes live inside the#ifdef _WIN32branch).wchar_tbuffer written byte-by-byte) both UTF-8 tests fail, on thecbm_utf8_to_widepath they pass.