26 KiB
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<ParseResult, std::string> 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_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
$<$<CONFIG:Debug>:-g -O0>
$<$<CONFIG:Release>:-O3>)
if(BUILD_TESTS)
add_test(NAME test_args_parser COMMAND $<TARGET_FILE:test_args_parser>)
endif()
Create lsp-server/test/test_args_parser/main.cc:
import lsp.test.args_parser;
int main()
{
return Run();
}
Create lsp-server/test/test_args_parser/test_args_parser.cppm:
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<std::string_view> arguments)
{
std::vector<std::string> storage{ "tsl-server" };
storage.reserve(arguments.size() + 1);
for (auto argument : arguments)
storage.emplace_back(argument);
std::vector<char*> argv;
argv.reserve(storage.size());
for (auto& argument : storage)
argv.push_back(argument.data());
return lsp::utils::ParseArgs(static_cast<int>(argv.size()), argv.data());
}
void ExpectError(std::initializer_list<std::string_view> 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:
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:
#!/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:
add_test(NAME test_cli_startup
COMMAND ${PYTHON3_EXECUTABLE}
${CMAKE_CURRENT_LIST_DIR}/test_cli_startup.py
--server $<TARGET_FILE:tsl-server>)
- Step 3: Run the new tests to verify the old implementation fails
Run:
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:
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<ParseResult, std::string> 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<spdlog::level::level_enum> 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<ParseResult, std::string> 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=<level> Set log level (trace, debug, info, warn, error, off)\n"
<< " --log-file=<path> Output logs to specified file\n"
<< " --threads=<count> Number of worker threads (1-256)\n"
<< " --interpreter=<path> Custom interpreter path\n";
}
}
- Step 5: Move logger ownership and staged errors into the launcher
Replace lsp-server/src/cli/launcher.cppm with:
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:
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
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:
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:
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:
std::string command = "\"" + server_path.string() + "\" --log=off < \"" +
input_path.string() + "\" > \"" + output_path.string() + "\"";
- Step 3: Format and rerun the affected tests
Run:
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
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:
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:
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:
"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:
"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:
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
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:
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:
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:
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:
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:
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.