- Update DRH_JSON_FROM_REQUIRED macro to use json_to_field() for proper nested type error handling
- Update DRH_JSON_CHECK_OPTIONAL macro to use json_to_field() for proper nested type error handling
- Add DRH_JSON_FROM_OPTIONAL macro for from_json (void return type) since DRH_JSON_CHECK_OPTIONAL now returns std::expected
- Update DRH_DEFINE_TYPE_OPTIONAL to use DRH_JSON_FROM_OPTIONAL for from_json
Note: json_to_field() is used without namespace prefix to comply with CONTRIBUTING.md no-namespaces policy.
The CONTRIBUTING.md policy states: 'This project does not use namespaces
in C++ code.' Move json_to_field to global scope and update test struct
to call it directly without namespace qualification.
- Add json_to_field<T>() helper in detail namespace for compile-time dispatch
- Uses if constexpr(has_json_to_v<T>) to choose between recursive json_to<T>()
for custom types and direct .get_to() for primitives
- Custom types: calls json_to<T>() and prepends field name to error messages
- Primitive types: uses .get_to() directly (no recursion)
- Add MixedFieldsStruct test type with both primitive and custom type fields
- Add test_json_to_field.sh test suite to verify correct dispatch and error messages
- Add helper class template detail::json_to_impl with partial specialization
for std::vector<T> when T has json_to<T>() specialization
- Primary json_to<T> template delegates to helper class
- Explicit specializations (from macros/manual) take precedence
- Add json_to_vector<T>() convenience function for direct use
- Add -R vector<FunctionCall> test support in main.cpp
- Add test_json_type_vector.sh with comprehensive tests
Closes issue 66
The project CONTRIBUTING.md policy prohibits namespaces. Removed
namespace detail wrapper from has_json_to trait and all its
specializations, placing them at global scope instead.
Convert EditToolParams, ReadFileToolParams, SkillToolParams, and
AgentTaskParams from using json_to_##Type() function calls to the
json_to<Type>() template specialization syntax.
Updated files:
- tools/edit_tool.cpp
- tools/file_read_tool.cpp
- src/skill.cpp
- tools/agent_tool.cpp
- src/main.cpp (-R flag handler)
Convert hand-written json_to functions to template specializations for
complex types with mixed required/optional fields and nested types:
- ToolCall uses json_to<ToolCall>() and internally calls json_to<FunctionCall>()
- Message uses json_to<Message>() and internally calls json_to<ToolCall>()
- Choice uses json_to<Choice>() and internally calls json_to<Message>()
- ChatResponse json_to_ChatResponse() now calls json_to<Choice>() for nested types
- HarnessState uses json_to<HarnessState>() with nested Message and HarnessConfig
Update all call sites in src/harness.cpp (ollama_client.cpp), src/debug_client.cpp,
src/main.cpp to use json_to<T>() instead of json_to_T().
All 28 tests pass.
- Update DRH_DEFINE_TYPE_REQUIRED macro to generate template specialization
json_to<Type>() in addition to the existing json_to_##Type() function
- Add template specialization for ChatResponse
- Update main.cpp -R flag handler for FunctionCall, Usage, ModelInfo, ModelListResponse
- Update ollama_client.cpp to use json_to<ChatResponse>() and json_to<ModelListResponse>()
- Update json_to_ChatResponse to use json_to<Usage>() for nested Usage field
All 28 tests pass.
Update ListFilesToolParams, GrepToolParams, and ShellToolParams
call sites to use the template json_to<T>() function instead of
the old json_to_##Type() style.
- tools/file_list_tool.cpp: Use json_to<ListFilesToolParams>()
- tools/grep_tool.cpp: Use json_to<GrepToolParams>()
- tools/shell_tool.cpp: Use json_to<ShellToolParams>()
- src/main.cpp: Update -R flag handler for these three types
- Add template forward declaration json_to<T>() in json_types.hpp
- Update DRH_DEFINE_TYPE_OPTIONAL macro to generate template specialization
json_to<Type>() while keeping the old json_to_##Type() function
- Update main.cpp -R flag handler for HarnessConfig to use json_to<HarnessConfig>()
Closes issue #59
- Add make_regex_safely() helper to json_types.hpp that wraps std::regex
construction and returns std::expected<std::regex, std::string>
- Replace try/catch for regex compilation with make_regex_safely()
- Replace try/catch for filesystem operations with std::error_code overloads
(following the pattern in skill.cpp)
All acceptance criteria met:
- No try/catch blocks in tools/grep_tool.cpp
- Invalid regex patterns produce clear error messages
- Filesystem errors during directory iteration produce clear error messages
- test_tool_grep.sh passes
- No throw statements in modified file
Replace the try/catch block in collect_paths() lambda with explicit
std::error_code-based error handling using recursive_directory_iterator
constructor and increment() methods that don't throw exceptions.
Error messages include the path that failed and the underlying error
message from the filesystem operation.
Added test_skill_unreadable_dir.sh to verify proper error handling when
a directory is unreadable.
Change the Tool interface to support error handling during parameter conversion:
- ToolParamsConverter now returns std::expected<std::any, std::string>
- ToolRegistry::invoke() handles conversion errors and returns them as tool result strings
- Replace NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE with DRH_DEFINE_TYPE_REQUIRED for SkillToolParams and AgentTaskParams
- edit_tool uses json_to_EditToolParams() instead of args.get<EditToolParams>()
- agent_tool uses json_to_AgentTaskParams() instead of args.get<AgentTaskParams>()
- Add test for missing required field error handling
Add a new Error Handling section documenting the project's error handling
conventions using std::expected:
- JSON parsing must use parse_json_safely()
- JSON deserialization must use json_to<T>()
- No try/catch blocks — handle std::expected explicitly
- No new exceptions allowed
- Error handling uses std::expected<T, std::string> exclusively
- Library code returns std::expected; expect_or_exit() for top-level code only
Also update C++ version requirement from C++20 to C++23 in Prerequisites.
Fixes: #53
Replace try/catch block in -p (print_file) mode with explicit
std::expected-style error checking using nlohmann::json::parse()
with the allow_discarded parameter and is_discarded() check.
This follows the pattern already used elsewhere in main.cpp
(line 541) for loading chat files.
- Updated Harness::chat() to return std::expected<std::string, std::string>
- Client errors are retried up to 20 times, then returns 'max retries exceeded'
- Tool invocation errors are returned as tool result string to LLM
- Max tool iterations returns 'max tool iterations exceeded' error
- Removed try/catch block for JSON parsing, using parse_json_safely() instead
- Updated main.cpp to handle expected return type from chat()
- Updated agent_tool.cpp to handle expected return type from chat()
- Added tests for client retry exhaustion, max tool iterations, and tool invocation errors
- parse_skill_frontmatter() now returns std::expected<SkillMetadata, std::string>
- scan_skills() now returns std::expected<std::vector<SkillMetadata>, std::string>
- All throw statements replaced with return std::unexpected()
- Filesystem scan errors are now fatal instead of warnings
- Updated main.cpp to handle the new return types
Update run_editor() to return std::expected<std::string, std::string>
for all editor-related failures instead of throwing exceptions.
Changes:
- include/editor.hpp: Change return type to std::expected
- src/editor.cpp: Replace all throw std::runtime_error() with
return std::unexpected() for error cases:
* Temp file open failure
* No nvim/ set
* Editor exit error
* Scissors separator not found
* Empty prompt after separator
- src/main.cpp: Use expect_or_exit() instead of try/catch
- register_tool() returns std::expected<void, std::string> for duplicate detection
- invoke() returns std::expected<std::string, std::string> for unknown tool errors
- create_subregistry() returns std::expected<ToolRegistry, std::string> for missing tools
- All throw statements removed from tool_registry.cpp
- Updated main.cpp to use expect_or_exit() for startup tool registration
- Updated harness.cpp and agent_tool.cpp to handle new return types
All 26 existing tests pass.
Update the Client abstract interface and its implementations to return
std::expected instead of throwing exceptions.
Changes:
- Client interface: chat() and list_models() now return std::expected
- OllamaClient: replaced all throw statements with return std::unexpected()
- DebugClient: replaced all throw statements with return std::unexpected()
- Removed OllamaClientError custom exception class
- Harness: updated to handle std::expected returns from Client
- main.cpp: updated list_models() call to handle std::expected
All 26 existing tests pass.
- Add json_to<Message>() with role required, other fields optional
- Add json_to<Choice>() with message required, finish_reason optional
- Add json_to<HarnessState>() with history/tools required, config optional
- Update json_to_ChatResponse() to use json_to_Choice() for nested types
- Remove from_json() functions for Message, Choice, HarnessState, ChatResponse
- Update ollama_client.cpp and debug_client.cpp to use json_to_* functions
- Update main.cpp to use json_to_HarnessState() for loading chat files
- Add test cases for Message, Choice, and HarnessState types
- Add test_json_type_mixed.sh test file
- Add DRH_DEFINE_TYPE_OPTIONAL macro for structs with all optional fields
- Missing JSON fields use struct member defaults
- Convert HarnessConfig, ListFilesToolParams, GrepToolParams, ShellToolParams
from NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE[_WITH_DEFAULT] to new macro
- Add test handling for new types in main.cpp -R flag
- Add comprehensive tests for optional type handling
Closes issue 45
- Upgrade CMakeLists.txt from C++20 to C++23 to enable std::expected
- Add parse_json_safely() to json_types.hpp: wraps json::parse with
allow_exceptions=false, returns discarded JSON on parse failure
- Add expect_or_exit() to json_types.hpp: helper for std::expected that
prints error to stderr and exits on failure
- Add test flags (-J, -E, -e) to CLI for testing the new functions
- Add bash tests for parse_json_safely() and expect_or_exit()
The bug was in src/main.cpp where temp_file_path.string().c_str() was called
on a temporary std::string object. The temporary was destroyed at the end of
the statement, leaving file_to_open pointing to deallocated memory.
Fixed by assigning to auto_temp_file first (which extends the lifetime), then
taking its c_str() for file_to_open.
- Added UUID v4 generation using <random> and <filesystem> headers
- When -n is specified without -F, create a temp file in system temp dir
- Temp file is named harness-<uuid>.json
- Print temp file path to stderr when session exits
- Updated help message to reflect new behavior
- Added test_auto_temp_file.sh to verify:
- Temp file creation with -n alone
- Path printed to stderr
- Existing -F <file> -n behavior preserved
- UUID uniqueness
- Changed skill_map to store SkillMetadata by value instead of using shared_ptr to pointers
- Removed unnecessary #include <memory>
- Updated lambda capture to move the map by value
- All unit tests pass
- Register skill tool as 'skill' instead of 'skill_tool'
- Change parameter name from 'skill_name' to 'name'
- Update XML output to include name attribute: <skill_content name="...">
- Update test_tool_skill.sh and test_skill_filter.sh to reflect new names
- Fixed Bash redirection bug () that prevented capturing stderr output.
- Replaced with which correctly captures stderr (since stdout is empty on failure).
- Ensures validation checks for clean error messages and non-zero exit codes work as intended.
- Wrap skill discovery and validation in a try-catch block to intercept std::runtime_error.
- Print a clean diagnostic message to stderr (e.g., 'Fatal: Malformed SKILL.md at <path>').
- Exit immediately with code 1 instead of crashing with a C++ stack trace.
- Update tests/test_skill_validation.sh to verify the clean error message and exit code.
Consolidate skill_parsing, skill_scanner, and skill_tool modules into
a single skill.hpp / skill.cpp pair as specified in issue #16.
Changes:
- Created include/skill.hpp with all declarations
- Created src/skill.cpp with all implementations
- Updated CMakeLists.txt to use the new consolidated file
- Updated src/main.cpp to include the new consolidated header
- Removed old files: skill_parsing.hpp/cpp, skill_scanner.hpp/cpp,
skill_tool.hpp/cpp
All 21 tests pass without regression.
- Replace SkillScanner class with free functions (scan_skills, print_discovered_paths, print_skills)
- Extend SkillMetadata with raw_content, files, and directory fields
- Update parse_skill_frontmatter to extract raw markdown content after YAML frontmatter
- Refactor make_skill_tool to take std::vector<SkillMetadata> and perform minimal logic (lookup and format pre-computed data)
- Update main.cpp to use new free functions
- All 20 tests pass
- Updated make_skill_tool to accept a descriptions map
- Modified skill_tool.cpp to build a comprehensive description listing all available skills and their descriptions
- Updated main.cpp to populate and pass the descriptions map
- Updated test_tool_skill.sh to verify tool execution with loaded skills
- Remove fallback behavior for malformed SKILL.md files in skill_scanner.cpp
- Now harness crashes immediately on invalid YAML frontmatter
- Updated tests to use proper YAML frontmatter
- Added new test_skill_validation.sh to verify crash behavior
- Removed test that expected fallback for invalid frontmatter
- Add flag to filter the list of enabled skills.
- Updated argument parsing to handle flag multiple times.
- Modified construction to filter skills based on provided names.
- Added test to verify non-loaded tools error when called.
- Updated README.md to document the new flag.
- Registered new test in tests/CMakeLists.txt.
darthrubik