plugin: add top-5 missing tests + FFI edge cases (R2 retargeted) - #42
Merged
Conversation
Coverage gaps the plugin subsystem audit flagged:
* `test_load_file_missing_path_returns_err` — load_file on a nonexistent
path returns Err with "Failed to read plugin" in the message, not a
panic or a silent success.
* `test_store_response_round_trips_via_harness_var` +
`test_store_response_escapes_special_chars` — the write side of the
harness-response slot. Special chars (quotes, newlines, backslashes)
round-trip cleanly.
* `test_concurrent_dispatch_tool_hook_serializes` — 8 threads share an
Arc<Mutex<PluginManager>> and each fire a tool hook with a distinct
:tool key. Without serialization a thread could observe another's
block reason; the test asserts each thread sees exactly its own.
* `on_tool_end_fires_when_inner_returns_error` (in hook.rs) — uses an
AlwaysFail fake tool to verify the wrapper still dispatches
on-tool-end after an inner Err, letting the plugin substitute a
replacement output. Pins the symmetric start/end contract against
accidental refactors.
FFI edge cases caught up to:
* `confirm_accepts_keyword_title` — `(harness/__confirm :warn ...)`
works; the cfn's is_keyword branch in `read_string_arg` is exercised.
* `select_with_empty_options_returns_nil` — empty options array returns
nil instead of emitting a phantom dialog request.
* `select_accepts_tuple_options` — quoted tuple `'("a" "b")` works the
same as array literal `["a" "b"]`; `read_string_array_arg` handles both.
* `select_returns_multibyte_option_through_wrap_string` — emoji + CJK +
Cyrillic in the reply round-trip through wrap_string's UTF-8 byte
count. Off-by-one in the i32::try_from path would lose bytes here.
Total: 513 pass with plugin (was 504 after R1). Non-plugin baseline
unchanged at 452 / 12 pre-existing.
One audited gap left unaddressed: forced Janet-init-failure path of
`Worker::try_spawn`. Janet init only fails on linker/library issues
which the test harness can't synthesize without mocking janetrs. Filed
as a known-untested behavior; the init-timeout watchdog added in R1
bounds the worst case.
Refs dirge-f5m.
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…-code#42) Coverage gaps the plugin subsystem audit flagged: * `test_load_file_missing_path_returns_err` — load_file on a nonexistent path returns Err with "Failed to read plugin" in the message, not a panic or a silent success. * `test_store_response_round_trips_via_harness_var` + `test_store_response_escapes_special_chars` — the write side of the harness-response slot. Special chars (quotes, newlines, backslashes) round-trip cleanly. * `test_concurrent_dispatch_tool_hook_serializes` — 8 threads share an Arc<Mutex<PluginManager>> and each fire a tool hook with a distinct :tool key. Without serialization a thread could observe another's block reason; the test asserts each thread sees exactly its own. * `on_tool_end_fires_when_inner_returns_error` (in hook.rs) — uses an AlwaysFail fake tool to verify the wrapper still dispatches on-tool-end after an inner Err, letting the plugin substitute a replacement output. Pins the symmetric start/end contract against accidental refactors. FFI edge cases caught up to: * `confirm_accepts_keyword_title` — `(harness/__confirm :warn ...)` works; the cfn's is_keyword branch in `read_string_arg` is exercised. * `select_with_empty_options_returns_nil` — empty options array returns nil instead of emitting a phantom dialog request. * `select_accepts_tuple_options` — quoted tuple `'("a" "b")` works the same as array literal `["a" "b"]`; `read_string_array_arg` handles both. * `select_returns_multibyte_option_through_wrap_string` — emoji + CJK + Cyrillic in the reply round-trip through wrap_string's UTF-8 byte count. Off-by-one in the i32::try_from path would lose bytes here. Total: 513 pass with plugin (was 504 after R1). Non-plugin baseline unchanged at 452 / 12 pre-existing. One audited gap left unaddressed: forced Janet-init-failure path of `Worker::try_spawn`. Janet init only fails on linker/library issues which the test harness can't synthesize without mocking janetrs. Filed as a known-untested behavior; the init-timeout watchdog added in R1 bounds the worst case. Refs dirge-f5m. Co-authored-by: Yogthos <yogthos@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
1 participant
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.
Re-targeting PR #36 against current main. The original PR landed in
`feat/plugin-critical-fixes` (its declared base) but didn't propagate
to main when R1 was squash-merged.
Content identical to #36 — the cherry-pick of b95ded6.