# Args Parser Startup Errors Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Refactor argument parsing into a stateless, strictly validated API and make all startup failures exit predictably without contaminating LSP stdout. **Architecture:** `lsp.utils.args_parser` becomes a pure parser that returns `std::expected` and owns only CLI syntax plus help rendering. `lsp.cli.launcher` owns logger initialization and maps argument, logger, and server failures to exit codes 2, 1, and 1. Repository callers are migrated away from the removed stdout/stderr selection flags. **Tech Stack:** C++23 Modules, `std::expected`, `std::from_chars`, spdlog, CMake/CTest, Python 3 subprocess tests, TypeScript --- ## Plan Meta - **Plan Group:** args-parser-startup-errors - **Parent Plan:** none - **Verification Scope:** parser unit tests, CLI startup tests, LSP JSON/provider tests, VSCode TypeScript compile - **Verification Gate:** all scoped CTest cases pass, VSCode compiles, and removed flags have no remaining runtime callers - **Execution Constraints:** `karpathy-guidelines`, `.agents`, `AGENT_RULES.md`, test-driven development ## File Map - Modify `lsp-server/src/utils/args_parser.cppm`: stateless parsing types, validation, and help rendering. - Modify `lsp-server/src/cli/launcher.cppm`: logger initialization and staged startup errors. - Create `lsp-server/test/test_args_parser/CMakeLists.txt`: parser unit-test target. - Create `lsp-server/test/test_args_parser/main.cc`: parser unit-test entry point. - Create `lsp-server/test/test_args_parser/test_args_parser.cppm`: parser and help tests. - Create `lsp-server/test/test_cli_startup.py`: executable-level startup regression tests. - Modify `lsp-server/test/CMakeLists.txt`: register the new C++ and Python tests. - Modify `lsp-server/test/run_lsp_json_tests.py`: remove obsolete logger-selection argument. - Modify `lsp-server/test/test_provider/server_json_test.cppm`: remove obsolete stdio alias. - Modify `vscode/src/extension.ts`: stop injecting the removed stderr flag. - Modify `vscode/package.json`: update default server arguments. - Modify `vscode/README.md`: document current arguments. - Modify `vim/README.md`: remove obsolete logger-selection examples while leaving interpreter-path expansion out of scope. ### Task 1: Drive the parser and launcher refactor with regression tests **Files:** - Create: `lsp-server/test/test_args_parser/CMakeLists.txt` - Create: `lsp-server/test/test_args_parser/main.cc` - Create: `lsp-server/test/test_args_parser/test_args_parser.cppm` - Create: `lsp-server/test/test_cli_startup.py` - Modify: `lsp-server/test/CMakeLists.txt` - Modify: `lsp-server/src/utils/args_parser.cppm` - Modify: `lsp-server/src/cli/launcher.cppm` - [ ] **Step 1: Add the parser unit-test target and failing tests** Create `lsp-server/test/test_args_parser/CMakeLists.txt`: ```cmake cmake_minimum_required(VERSION 4.0) project(test_args_parser LANGUAGES CXX) set(CMAKE_CXX_STANDARD 23) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP 1) find_package(spdlog CONFIG REQUIRED) find_package(fmt CONFIG REQUIRED) add_executable(test_args_parser main.cc test_args_parser.cppm) if(TARGET std_module) add_dependencies(test_args_parser std_module) endif() target_sources( test_args_parser PRIVATE FILE_SET cxx_modules TYPE CXX_MODULES BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../../src FILES test_args_parser.cppm ../../src/bridge/spdlog.cppm ../../src/utils/args_parser.cppm) target_compile_definitions(test_args_parser PRIVATE SPDLOG_HEADER_ONLY FMT_HEADER_ONLY) target_link_libraries(test_args_parser PRIVATE spdlog::spdlog_header_only fmt::fmt-header-only) target_compile_options( test_args_parser PRIVATE -Wall -Wextra -Wpedantic -Wno-import-implementation-partition-unit-in-interface-unit $<$:-g -O0> $<$:-O3>) if(BUILD_TESTS) add_test(NAME test_args_parser COMMAND $) endif() ``` Create `lsp-server/test/test_args_parser/main.cc`: ```cpp import lsp.test.args_parser; int main() { return Run(); } ``` Create `lsp-server/test/test_args_parser/test_args_parser.cppm`: ```cpp module; export module lsp.test.args_parser; import spdlog; import std; import lsp.utils.args_parser; namespace { void Expect(bool condition, std::string_view message) { if (!condition) throw std::runtime_error(std::string(message)); } auto Parse(std::initializer_list arguments) { std::vector storage{ "tsl-server" }; storage.reserve(arguments.size() + 1); for (auto argument : arguments) storage.emplace_back(argument); std::vector argv; argv.reserve(storage.size()); for (auto& argument : storage) argv.push_back(argument.data()); return lsp::utils::ParseArgs(static_cast(argv.size()), argv.data()); } void ExpectError(std::initializer_list arguments, std::string_view fragment) { auto result = Parse(arguments); Expect(!result.has_value(), "arguments should be rejected"); Expect(result.error().find(fragment) != std::string::npos, "error should identify the invalid argument"); } } export int Run() { int failures = 0; auto RunCase = [&failures](std::string_view name, auto test) { try { test(); std::cout << "[PASS] " << name << '\n'; } catch (const std::exception& error) { ++failures; std::cout << "[FAIL] " << name << ": " << error.what() << '\n'; } }; RunCase("defaults", [] { auto result = Parse({}); Expect(result.has_value(), "default arguments should parse"); Expect(result->action == lsp::utils::ParseAction::kRun, "default action should run"); Expect(result->config.thread_count == 4, "default thread count should be 4"); Expect(result->config.log_level == spdlog::level::info, "default log level should be info"); }); RunCase("valid values and last occurrence", [] { auto result = Parse({ "--threads=8", "--threads=16", "--log=debug", "--log-file=server.log", "--interpreter=/opt/tsl" }); Expect(result.has_value(), "valid arguments should parse"); Expect(result->config.thread_count == 16, "last thread count should win"); Expect(result->config.log_level == spdlog::level::debug, "debug log level should parse"); Expect(result->config.log_file == "server.log", "log file should parse"); Expect(result->config.interpreter_path == "/opt/tsl", "interpreter path should parse"); }); RunCase("all log levels", [] { const std::array levels{ std::pair{ "trace", spdlog::level::trace }, std::pair{ "debug", spdlog::level::debug }, std::pair{ "info", spdlog::level::info }, std::pair{ "warn", spdlog::level::warn }, std::pair{ "error", spdlog::level::err }, std::pair{ "off", spdlog::level::off }, }; for (const auto& [name, level] : levels) { const std::string argument = std::string("--log=") + name; auto result = Parse({ argument }); Expect(result.has_value(), "known log level should parse"); Expect(result->config.log_level == level, "log level should match"); } }); RunCase("help takes precedence", [] { auto result = Parse({ "--unknown", "--help", "--threads=bad" }); Expect(result.has_value(), "help should bypass other validation"); Expect(result->action == lsp::utils::ParseAction::kShowHelp, "help action should be returned"); }); RunCase("thread validation", [] { Expect(Parse({ "--threads=1" }).has_value(), "one thread should parse"); Expect(Parse({ "--threads=256" }).has_value(), "256 threads should parse"); for (auto argument : { "--threads=", "--threads=0", "--threads=-1", "--threads=257", "--threads=12junk", "--threads=18446744073709551616" }) ExpectError({ argument }, "--threads"); }); RunCase("other invalid values", [] { for (auto argument : { "--log=verbose", "--log-file=", "--interpreter=", "--log-stdout", "--log-stderr", "--use-stdio", "--unknown" }) ExpectError({ argument }, "--"); }); RunCase("help formatting", [] { std::ostringstream output; lsp::utils::PrintHelp(output, "tsl-server"); const auto text = output.str(); Expect(text.contains("Usage: tsl-server [options]\n\nOptions:\n"), "help should contain physical line breaks"); Expect(!text.contains("\\n"), "help should not contain escaped newline text"); Expect(!text.contains("--log-stdout"), "removed stdout flag should be absent"); Expect(!text.contains("--log-stderr"), "removed stderr flag should be absent"); Expect(!text.contains("--use-stdio"), "removed stdio alias should be absent"); }); std::cout << "Failures: " << failures << '\n'; return failures == 0 ? 0 : 1; } ``` Register the directory in `lsp-server/test/CMakeLists.txt` after `test_scheduler`: ```cmake if(EXISTS ${CMAKE_CURRENT_LIST_DIR}/test_args_parser/CMakeLists.txt) add_subdirectory(test_args_parser) endif() ``` - [ ] **Step 2: Add failing executable-level startup tests** Create `lsp-server/test/test_cli_startup.py`: ```python #!/usr/bin/env python3 import argparse import subprocess import tempfile import unittest from pathlib import Path SERVER: Path def run_server(*arguments: str) -> subprocess.CompletedProcess[bytes]: return subprocess.run( [str(SERVER), *arguments], input=b"", capture_output=True, check=False, ) class CliStartupTest(unittest.TestCase): def test_help_uses_real_newlines(self) -> None: result = run_server("--help") self.assertEqual(0, result.returncode) self.assertIn(b"\n\nOptions:\n", result.stdout) self.assertNotIn(b"\\n", result.stdout) def test_invalid_threads_return_argument_error(self) -> None: for argument in ("--threads=", "--threads=-1", "--threads=12junk"): with self.subTest(argument=argument): result = run_server(argument) self.assertEqual(2, result.returncode) self.assertIn(b"--threads", result.stderr) self.assertNotIn(b"TSL-LSP server starting", result.stderr) def test_removed_stdout_flag_is_rejected(self) -> None: result = run_server("--log-stdout") self.assertEqual(2, result.returncode) self.assertEqual(b"", result.stdout) def test_default_logger_never_writes_stdout(self) -> None: result = run_server() self.assertEqual(0, result.returncode) self.assertEqual(b"", result.stdout) self.assertIn(b"TSL-LSP server starting", result.stderr) def test_log_file_failure_is_controlled(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: path = Path(temp_dir) / "missing" / "server.log" result = run_server(f"--log-file={path}") self.assertEqual(1, result.returncode) self.assertIn(b"Failed to initialize logger", result.stderr) if __name__ == "__main__": parser = argparse.ArgumentParser() parser.add_argument("--server", type=Path, required=True) args, unittest_args = parser.parse_known_args() SERVER = args.server.resolve() unittest.main(argv=[__file__, *unittest_args]) ``` Register it next to `test_lsp_json` in `lsp-server/test/CMakeLists.txt`: ```cmake add_test(NAME test_cli_startup COMMAND ${PYTHON3_EXECUTABLE} ${CMAKE_CURRENT_LIST_DIR}/test_cli_startup.py --server $) ``` - [ ] **Step 3: Run the new tests to verify the old implementation fails** Run: ```bash cmake -S lsp-server -B lsp-server/build/clang-linux/Release cmake --build lsp-server/build/clang-linux/Release --target tsl-server python lsp-server/test/test_cli_startup.py \ --server lsp-server/build/clang-linux/Release/src/tsl-server cmake --build lsp-server/build/clang-linux/Release --target test_args_parser ``` Expected: the Python suite fails the escaped-newline and controlled-error assertions. The subsequent parser-test build fails because `ParseArgs`, `ParseAction`, and the stream-based `PrintHelp` do not exist. - [ ] **Step 4: Replace the stateful parser with the stateless implementation** Replace `lsp-server/src/utils/args_parser.cppm` with: ```cpp module; export module lsp.utils.args_parser; import spdlog; import std; export namespace lsp::utils { struct ServerConfig { std::size_t thread_count = 4; spdlog::level::level_enum log_level = spdlog::level::info; std::string log_file; std::string interpreter_path; }; enum class ParseAction { kRun, kShowHelp, }; struct ParseResult { ParseAction action = ParseAction::kRun; ServerConfig config; }; std::expected ParseArgs(int argc, char* const argv[]); void PrintHelp(std::ostream& output, std::string_view program_name); } namespace lsp::utils { namespace { constexpr std::size_t kMinThreadCount = 1; constexpr std::size_t kMaxThreadCount = 256; std::optional ParseLogLevel(std::string_view value) { if (value == "trace") return spdlog::level::trace; if (value == "debug") return spdlog::level::debug; if (value == "info") return spdlog::level::info; if (value == "warn") return spdlog::level::warn; if (value == "error") return spdlog::level::err; if (value == "off") return spdlog::level::off; return std::nullopt; } } std::expected ParseArgs(int argc, char* const argv[]) { for (int i = 1; i < argc; ++i) { if (std::string_view(argv[i]) == "--help") return ParseResult{ .action = ParseAction::kShowHelp }; } ParseResult result; constexpr std::string_view kLogPrefix = "--log="; constexpr std::string_view kLogFilePrefix = "--log-file="; constexpr std::string_view kThreadsPrefix = "--threads="; constexpr std::string_view kInterpreterPrefix = "--interpreter="; for (int i = 1; i < argc; ++i) { const std::string_view argument = argv[i]; if (argument.starts_with(kLogPrefix)) { const auto value = argument.substr(kLogPrefix.size()); auto level = ParseLogLevel(value); if (!level) return std::unexpected("Invalid value for --log: '" + std::string(value) + "'"); result.config.log_level = *level; } else if (argument.starts_with(kLogFilePrefix)) { const auto value = argument.substr(kLogFilePrefix.size()); if (value.empty()) return std::unexpected("--log-file requires a non-empty path"); result.config.log_file = value; } else if (argument.starts_with(kThreadsPrefix)) { const auto value = argument.substr(kThreadsPrefix.size()); std::size_t count = 0; const auto [end, error] = std::from_chars(value.data(), value.data() + value.size(), count); if (error != std::errc{} || end != value.data() + value.size() || count < kMinThreadCount || count > kMaxThreadCount) { return std::unexpected("Invalid value for --threads: '" + std::string(value) + "' (expected 1..256)"); } result.config.thread_count = count; } else if (argument.starts_with(kInterpreterPrefix)) { const auto value = argument.substr(kInterpreterPrefix.size()); if (value.empty()) return std::unexpected("--interpreter requires a non-empty path"); result.config.interpreter_path = value; } else { return std::unexpected("Unknown argument: " + std::string(argument)); } } return result; } void PrintHelp(std::ostream& output, std::string_view program_name) { output << "Usage: " << program_name << " [options]\n\n" << "Options:\n" << " --help Show this help message\n" << " --log= Set log level (trace, debug, info, warn, error, off)\n" << " --log-file= Output logs to specified file\n" << " --threads= Number of worker threads (1-256)\n" << " --interpreter= Custom interpreter path\n"; } } ``` - [ ] **Step 5: Move logger ownership and staged errors into the launcher** Replace `lsp-server/src/cli/launcher.cppm` with: ```cpp module; export module lsp.cli.launcher; import spdlog; import std; import lsp.core.server; import lsp.utils.args_parser; namespace { void SetupLogger(const lsp::utils::ServerConfig& config) { spdlog::set_pattern("%Y-%m-%d %H:%M:%S.%e [%t] [%^%l%$] %v"); auto logger = config.log_file.empty() ? spdlog::stderr_logger_mt("console_logger") : spdlog::basic_logger_mt("file_logger", config.log_file); logger->set_level(config.log_level); spdlog::set_default_logger(logger); spdlog::set_level(config.log_level); } } export int Run(int argc, char* argv[]) { auto parsed = lsp::utils::ParseArgs(argc, argv); if (!parsed) { std::cerr << "[TSL-LSP] Argument error: " << parsed.error() << '\n'; return 2; } if (parsed->action == lsp::utils::ParseAction::kShowHelp) { lsp::utils::PrintHelp(std::cout, argv[0]); return 0; } const auto& config = parsed->config; try { SetupLogger(config); } catch (const std::exception& error) { std::cerr << "[TSL-LSP] Failed to initialize logger: " << error.what() << '\n'; return 1; } try { spdlog::info("TSL-LSP server starting..."); lsp::core::LspServer server(config.thread_count, config.interpreter_path); server.Run(); } catch (const std::exception& error) { std::cerr << "[TSL-LSP] Server fatal error: " << error.what() << '\n'; spdlog::error("Server fatal error: {}", error.what()); spdlog::shutdown(); return 1; } catch (...) { std::cerr << "[TSL-LSP] Server unknown fatal error\n"; spdlog::error("Server unknown fatal error"); spdlog::shutdown(); return 1; } spdlog::info("TSL-LSP server stopped normally"); spdlog::shutdown(); return 0; } ``` - [ ] **Step 6: Format, build, and run the new tests** Run: ```bash clang-format -i \ lsp-server/src/utils/args_parser.cppm \ lsp-server/src/cli/launcher.cppm \ lsp-server/test/test_args_parser/main.cc \ lsp-server/test/test_args_parser/test_args_parser.cppm cmake -S lsp-server -B lsp-server/build/clang-linux/Release cmake --build lsp-server/build/clang-linux/Release \ --target tsl-server test_args_parser ctest --test-dir lsp-server/build/clang-linux/Release \ -R 'test_args_parser|test_cli_startup' --output-on-failure ``` Expected: both tests pass; malformed arguments return 2, logger creation failure returns 1, and default logs do not appear on stdout. - [ ] **Step 7: Commit the parser and launcher refactor** ```bash git add lsp-server/src/utils/args_parser.cppm \ lsp-server/src/cli/launcher.cppm \ lsp-server/test/CMakeLists.txt \ lsp-server/test/test_args_parser \ lsp-server/test/test_cli_startup.py git commit -m "refactor(cli): validate startup arguments" ``` ### Task 2: Migrate LSP test callers from removed flags **Files:** - Modify: `lsp-server/test/run_lsp_json_tests.py` - Modify: `lsp-server/test/test_provider/server_json_test.cppm` - [ ] **Step 1: Run affected tests to expose obsolete arguments** Run: ```bash ctest --test-dir lsp-server/build/clang-linux/Release \ -R 'test_lsp_json|test_provider' --output-on-failure ``` Expected: `test_lsp_json` fails because `--log-stderr` is unknown, and the provider server subprocess case fails because `--use-stdio` is unknown. - [ ] **Step 2: Remove the obsolete arguments from both launch sites** Change `lsp-server/test/run_lsp_json_tests.py` to: ```python proc = subprocess.Popen( [str(server_path), "--log=off"], stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) ``` Change the command construction in `lsp-server/test/test_provider/server_json_test.cppm` to: ```cpp std::string command = "\"" + server_path.string() + "\" --log=off < \"" + input_path.string() + "\" > \"" + output_path.string() + "\""; ``` - [ ] **Step 3: Format and rerun the affected tests** Run: ```bash clang-format -i lsp-server/test/test_provider/server_json_test.cppm cmake --build lsp-server/build/clang-linux/Release --target test_provider ctest --test-dir lsp-server/build/clang-linux/Release \ -R 'test_lsp_json|test_provider' --output-on-failure ``` Expected: both tests pass. - [ ] **Step 4: Commit the LSP test-call migration** ```bash git add lsp-server/test/run_lsp_json_tests.py \ lsp-server/test/test_provider/server_json_test.cppm git commit -m "test(lsp): remove obsolete logger flags" ``` ### Task 3: Update editor integrations and user documentation **Files:** - Modify: `vscode/src/extension.ts` - Modify: `vscode/package.json` - Modify: `vscode/README.md` - Modify: `vim/README.md` - [ ] **Step 1: Confirm editor integrations still reference removed flags** Run: ```bash rg -n --glob '!node_modules/**' -- \ '--log-stdout|--log-stderr|--use-stdio|--log=stderr' vscode vim ``` Expected: matches appear in `vscode/src/extension.ts`, `vscode/package.json`, both editor READMEs, and nowhere else under these directories. - [ ] **Step 2: Stop VSCode from injecting the removed flag** Change the argument setup in `vscode/src/extension.ts` to: ```typescript const runArgs = [...serverArguments] const debugArgs = serverArguments.map(arg => arg.startsWith('--log=') ? '--log=trace' : arg) if (!debugArgs.some(arg => arg.startsWith('--log='))) debugArgs.push('--log=trace') ``` Change the `tsl.server.arguments` default in `vscode/package.json` to: ```json "default": [ "--log=info" ] ``` - [ ] **Step 3: Update documented examples without changing interpreter-path behavior** In `vscode/README.md`, use `["--log=info"]` for both the documented default and the configuration example. In `vim/README.md`, use this server argument list and remove the `--log-stderr` table row: ```json "args": ["--log=info", "--interpreter=~/tsl64"] ``` Do not change `~/tsl64` in this task; home-directory expansion is explicitly deferred by the design. - [ ] **Step 4: Compile VSCode and confirm removed flags are gone from runtime callers** Run: ```bash npm --prefix vscode run compile rg -n --glob '!node_modules/**' -- \ '--log-stdout|--log-stderr|--use-stdio|--log=stderr' \ vscode/src vscode/package.json vscode/README.md vim/README.md ``` Expected: TypeScript compilation succeeds and the search returns no matches. - [ ] **Step 5: Commit editor integration updates** ```bash git add vscode/src/extension.ts vscode/package.json vscode/README.md vim/README.md git commit -m "refactor(editors): use fixed stderr logging" ``` ### Task 4: Run the complete scoped verification gate **Files:** - Verify only; no planned source changes. - [ ] **Step 1: Verify formatting and whitespace** Run: ```bash clang-format --dry-run --Werror \ lsp-server/src/utils/args_parser.cppm \ lsp-server/src/cli/launcher.cppm \ lsp-server/test/test_args_parser/main.cc \ lsp-server/test/test_args_parser/test_args_parser.cppm \ lsp-server/test/test_provider/server_json_test.cppm git diff --check ``` Expected: both commands exit 0 with no diagnostics. - [ ] **Step 2: Build all affected C++ targets** Run: ```bash cmake --build lsp-server/build/clang-linux/Release \ --target tsl-server test_args_parser test_provider ``` Expected: all three targets build successfully. - [ ] **Step 3: Run all scoped CTest cases** Run: ```bash ctest --test-dir lsp-server/build/clang-linux/Release \ -R 'test_args_parser|test_cli_startup|test_lsp_json|test_provider' \ --output-on-failure ``` Expected: four tests run and all pass. - [ ] **Step 4: Verify the editor build and removed runtime arguments** Run: ```bash npm --prefix vscode run compile rg -n --glob '!node_modules/**' --glob '!docs/superpowers/**' -- \ '--log-stdout|--log-stderr|--use-stdio|--log=stderr' \ lsp-server/src lsp-server/test/run_lsp_json_tests.py \ lsp-server/test/test_provider vscode/src vscode/package.json \ vscode/README.md vim/README.md ``` Expected: TypeScript compilation succeeds. The search may match only negative assertions inside `test_args_parser.cppm`; it must not match a server invocation, editor argument list, help string, or documentation example. - [ ] **Step 5: Inspect final changes against the design** Run: ```bash git status --short git log --oneline -4 git diff HEAD~3 -- \ lsp-server/src/utils/args_parser.cppm \ lsp-server/src/cli/launcher.cppm \ lsp-server/test vscode/src/extension.ts vscode/package.json \ vscode/README.md vim/README.md ``` Expected: the diff contains only the planned parser, launcher, tests, caller migrations, and documentation updates; unrelated pre-existing worktree changes remain untouched.