feat(ydotool): add comprehensive unit tests for ydotool injector

[Coldaine]

User description

Summary

Rebased and integrated from Jules draft PR #273.

Changes

• Add TestHarness struct for controlled test environment with temp directories
• Add tests for socket discovery priority (runtime → user systemd → system systemd)
• Add tests for binary permission checks
• Add tests for text injection (paste vs type fallback)
• Gate test_ydotool_injector module behind #[cfg(all(unix, feature = "ydotool"))]

Testing

cargo test -p coldvox-text-injection --features ydotool

Closes #273

---

PR Type

Tests, Enhancement

---

Description

• Add comprehensive unit tests for ydotool injector module

• Implement TestHarness for isolated test environment with mocks

• Test socket discovery priority and binary permission checks

• Test text injection with paste/type fallback mechanism

• Make ydotool functions testable via UINPUT_PATH_OVERRIDE

---

Diagram Walkthrough

flowchart LR
  A["ydotool_injector.rs"] -->|expose functions| B["test_ydotool_injector.rs"]
  C["TestHarness"] -->|creates isolated env| B
  B -->|tests socket discovery| D["candidate_socket_paths"]
  B -->|tests binary checks| E["check_binary_permissions"]
  B -->|tests injection| F["inject_text"]
  G["UINPUT_PATH_OVERRIDE"] -->|enables testing| H["check_uinput_access"]

Loading

File Walkthrough

	Relevant files
Tests	mod.rs Register ydotool injector test module                                        crates/coldvox-text-injection/src/tests/mod.rs Register new test module test_ydotool_injector behind feature gate Gate module with #[cfg(all(unix, feature = "ydotool"))] for platform-specific tests +2/-0      test_ydotool_injector.rs Add comprehensive ydotool injector unit tests                        crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs Implement TestHarness struct for controlled test environment with temp directories Create mock binaries and socket files for isolated testing Add test for socket path discovery priority (custom → runtime → user systemd → system) Add test for socket location discovery finding first available socket Add test for binary permission checks with mock ydotool Add test for ydotool availability when binary and socket present Add test for text injection using paste mode by default Add test for fallback from paste to type mode when paste fails Use #[serial] attribute to prevent test interference +207/-0
Enhancement	ydotool_injector.rs Expose functions and add test-friendly uinput override      crates/coldvox-text-injection/src/ydotool_injector.rs Make candidate_socket_paths() public for testing via pub(crate) Make locate_existing_socket() public for testing via pub(crate) Add UINPUT_PATH_OVERRIDE environment variable support in check_uinput_access() Allow tests to override /dev/uinput path without requiring actual device permissions +9/-3

---

[qodo-free-for-open-source-projects]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Command injection vulnerability Description: The create_which_mock function creates a shell script that directly interpolates a path into the script content without proper escaping, allowing potential command injection if target_binary contains shell metacharacters. test_ydotool_injector.rs [64-68] Referred Code fn create_which_mock(&self, target_binary: &Path) -> Result<()> { let content = format!("#!/bin/sh\necho {}", target_binary.display()); self.create_mock_binary("which", &content, true)?; Ok(()) }
	Global state modification Description: Environment variables PATH, HOME, XDG_RUNTIME_DIR, and UINPUT_PATH_OVERRIDE are modified globally using env::set_var, which can affect other concurrently running tests and cause race conditions despite the #[serial] attribute on individual tests. test_ydotool_injector.rs [43-48] Referred Code let original_path = env::var("PATH").unwrap_or_default(); let new_path = format!("{}:{}", bin_dir.display(), original_path); env::set_var("PATH", new_path); env::set_var("HOME", &home_dir); env::set_var("XDG_RUNTIME_DIR", &runtime_dir); env::set_var("UINPUT_PATH_OVERRIDE", &uinput_path);
	Arbitrary file path override Description: The UINPUT_PATH_OVERRIDE environment variable allows arbitrary file path override in test mode, which could be exploited if test code runs in production or if the cfg!(test) check is bypassed, potentially allowing unauthorized file access. ydotool_injector.rs [199-203] Referred Code let uinput_path = if cfg!(test) { env::var("UINPUT_PATH_OVERRIDE").unwrap_or_else(|_| "/dev/uinput".to_string()) } else { "/dev/uinput".to_string() };
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Environment Variable Injection: The UINPUT_PATH_OVERRIDE environment variable allows arbitrary file path override without validation, which could be exploited if tests run in untrusted environments. Referred Code env::set_var("UINPUT_PATH_OVERRIDE", &uinput_path); Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Properly restore environment in tests In the TestHarness, save the original values of environment variables before modification and restore them in the drop implementation to ensure proper test isolation. crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs [16-102] +use std::ffi::OsString; +... struct TestHarness { _temp_dir: TempDir, bin_dir: PathBuf, home_dir: PathBuf, runtime_dir: PathBuf, original_path: String, + original_home: Option<OsString>, + original_xdg_runtime_dir: Option<OsString>, + original_uinput_override: Option<OsString>, /// Path to a file that mock binaries can use to report arguments. output_file: PathBuf, } impl TestHarness { fn new() -> Result<Self> { let temp_dir = tempdir()?; let base_path = temp_dir.path(); let bin_dir = base_path.join("bin"); let home_dir = base_path.join("home"); let runtime_dir = base_path.join("run"); let uinput_path = base_path.join("uinput"); let output_file = base_path.join("output.log"); fs::create_dir_all(&bin_dir)?; fs::create_dir_all(&home_dir)?; fs::create_dir_all(&runtime_dir)?; File::create(&uinput_path)?; File::create(&output_file)?; let original_path = env::var("PATH").unwrap_or_default(); let new_path = format!("{}:{}", bin_dir.display(), original_path); env::set_var("PATH", new_path); + + let original_home = env::var_os("HOME"); env::set_var("HOME", &home_dir); + + let original_xdg_runtime_dir = env::var_os("XDG_RUNTIME_DIR"); env::set_var("XDG_RUNTIME_DIR", &runtime_dir); + + let original_uinput_override = env::var_os("UINPUT_PATH_OVERRIDE"); env::set_var("UINPUT_PATH_OVERRIDE", &uinput_path); env::remove_var("YDOTOOL_SOCKET"); env::remove_var("UID"); Ok(Self { _temp_dir: temp_dir, bin_dir, home_dir, runtime_dir, original_path, + original_home, + original_xdg_runtime_dir, + original_uinput_override, output_file, }) } ... impl Drop for TestHarness { fn drop(&mut self) { env::set_var("PATH", &self.original_path); - env::remove_var("HOME"); - env::remove_var("XDG_RUNTIME_DIR"); + if let Some(home) = &self.original_home { + env::set_var("HOME", home); + } else { + env::remove_var("HOME"); + } + if let Some(xdg_runtime_dir) = &self.original_xdg_runtime_dir { + env::set_var("XDG_RUNTIME_DIR", xdg_runtime_dir); + } else { + env::remove_var("XDG_RUNTIME_DIR"); + } + if let Some(uinput_override) = &self.original_uinput_override { + env::set_var("UINPUT_PATH_OVERRIDE", uinput_override); + } else { + env::remove_var("UINPUT_PATH_OVERRIDE"); + } env::remove_var("YDOTOOL_SOCKET"); env::remove_var("UID"); - env::remove_var("UINPUT_PATH_OVERRIDE"); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a test isolation issue where environment variables are not restored, which could lead to flaky tests. The proposed fix is accurate and improves the robustness of the test suite.	Medium
Update

[qodo-code-review]

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Restore original environment variables Modify the TestHarness to save the original values of environment variables and restore them in the drop implementation, instead of unconditionally removing them, to prevent test side effects. crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs [93-102] impl Drop for TestHarness { fn drop(&mut self) { env::set_var("PATH", &self.original_path); - env::remove_var("HOME"); - env::remove_var("XDG_RUNTIME_DIR"); - env::remove_var("YDOTOOL_SOCKET"); - env::remove_var("UID"); - env::remove_var("UINPUT_PATH_OVERRIDE"); + if let Some(home) = &self.original_home { + env::set_var("HOME", home); + } else { + env::remove_var("HOME"); + } + if let Some(xdg) = &self.original_xdg_runtime_dir { + env::set_var("XDG_RUNTIME_DIR", xdg); + } else { + env::remove_var("XDG_RUNTIME_DIR"); + } + if let Some(socket) = &self.original_ydotool_socket { + env::set_var("YDOTOOL_SOCKET", socket); + } else { + env::remove_var("YDOTOOL_SOCKET"); + } + if let Some(uid) = &self.original_uid { + env::set_var("UID", uid); + } else { + env::remove_var("UID"); + } + if let Some(uinput) = &self.original_uinput_override { + env::set_var("UINPUT_PATH_OVERRIDE", uinput); + } else { + env::remove_var("UINPUT_PATH_OVERRIDE"); + } } } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that unconditionally removing environment variables is bad practice and can lead to flaky tests; restoring them to their original state is a significant improvement in test robustness.	Medium
	Prevent empty PATH entry Conditionally construct the new PATH string to avoid adding a leading colon if the PATH environment variable was not originally set. crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs [43-44] let original_path = env::var("PATH").unwrap_or_default(); -let new_path = format!("{}:{}", bin_dir.display(), original_path); +let new_path = if original_path.is_empty() { + bin_dir.display().to_string() +} else { + format!("{}:{}", bin_dir.display(), original_path) +}; Apply / Chat Suggestion importance[1-10]: 4 __ Why: This is a minor but correct improvement that handles an edge case where the PATH variable might be unset, preventing a malformed PATH string and making the test harness more robust.	Low
Possible issue	Fix incorrect logic in socket test Fix the test_locate_existing_socket_finds_first_available test by creating multiple mock sockets at different priority levels to correctly verify that the highest-priority socket is located. crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs [116-126] #[test] #[serial] fn test_locate_existing_socket_finds_first_available() { let harness = TestHarness::new().unwrap(); - let _ = harness.runtime_dir.join(".ydotool_socket"); + + // Create a socket with lower priority. + let lower_priority_socket = harness.runtime_dir.join("ydotool.sock"); + harness.create_mock_socket(&lower_priority_socket).unwrap(); + + // Create the expected socket with higher priority. let expected_socket = harness.home_dir.join(".ydotool").join("socket"); harness.create_mock_socket(&expected_socket).unwrap(); let located = locate_existing_socket(); assert_eq!(located, Some(expected_socket)); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a logical flaw in the test where it fails to check socket location priority, and the proposed change correctly implements the intended test logic, improving test suite correctness.	Medium
	Always use override uinput path Remove the cfg!(test) condition to allow the UINPUT_PATH_OVERRIDE environment variable to be used in all build configurations, not just during testing. crates/coldvox-text-injection/src/ydotool_injector.rs [199-206] -let uinput_path = if cfg!(test) { - env::var("UINPUT_PATH_OVERRIDE").unwrap_or_else(|_| "/dev/uinput".to_string()) -} else { - "/dev/uinput".to_string() -}; +let uinput_path = env::var("UINPUT_PATH_OVERRIDE") + .unwrap_or_else(|_| "/dev/uinput".to_string()); -match OpenOptions::new().write(true).open(uinput_path) { +match OpenOptions::new().write(true).open(&uinput_path) { Apply / Chat Suggestion importance[1-10]: 5 __ Why: This is a good suggestion that simplifies the code and increases flexibility by allowing the uinput path to be overridden in any build configuration, not just for tests.	Low
More

[Copilot]

Pull request overview

This PR adds comprehensive unit tests for the ydotool text injection backend, originally drafted in PR #273. The tests validate socket discovery mechanisms, binary permission checks, and text injection behavior with appropriate fallback handling.

Key Changes

• Adds TestHarness infrastructure for isolated test environments with temporary directories
• Implements tests covering socket discovery priority order (YDOTOOL_SOCKET → HOME → XDG_RUNTIME_DIR → /run/user/{uid})
• Validates binary permission checks and text injection methods (paste with type fallback)
• Makes internal functions pub(crate) to enable testing and adds test-specific hooks (e.g., UINPUT_PATH_OVERRIDE)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
crates/coldvox-text-injection/src/ydotool_injector.rs	Exposes internal functions for testing and adds UINPUT_PATH_OVERRIDE environment variable support for test isolation
crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs	Adds comprehensive unit tests with TestHarness for socket discovery, binary permissions, and text injection behavior
crates/coldvox-text-injection/src/tests/mod.rs	Gates new test module behind appropriate feature flags (unix + ydotool)

[Copilot]

The PathBuf created here is unused. This line appears to be a leftover from development. Either remove it if it's not needed, or create the socket at this path if the intent was to test that the HOME socket takes priority over the runtime directory socket.

Suggested change

	let _ = harness.runtime_dir.join(".ydotool_socket");
	let runtime_socket = harness.runtime_dir.join(".ydotool_socket");
	harness.create_mock_socket(&runtime_socket).unwrap();

[Copilot]

The TestHarness is created but only used for its Drop implementation to clean up environment variables. The test itself doesn't use any of the harness's fields or methods. Consider using a simpler cleanup approach with a guard struct that only stores original_path, or rename the variable to _harness to indicate it's intentionally unused except for its Drop implementation.

Suggested change

	let harness = TestHarness::new().unwrap();
	let _harness = TestHarness::new().unwrap();

[Copilot]

The InjectionContext import is unused in this test file. Remove it to clean up the imports.

Suggested change

	use crate::types::{InjectionConfig, InjectionContext};
	use crate::types::InjectionConfig;

[Copilot]

The ydotool_daemon_socket import is unused in this test file. Remove it to clean up the imports.

Suggested change

	candidate_socket_paths, locate_existing_socket, ydotool_daemon_socket, YdotoolInjector,
	candidate_socket_paths, locate_existing_socket, YdotoolInjector,

[qodo-code-review]

CI Feedback 🧐

(Feedback updated until commit 46a1988)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Unit Tests & Golden Master (Hosted) (stable)

Failed stage: Run clippy [❌]

Failure summary: The action failed due to compilation errors in the coldvox-text-injection crate during the cargo clippy check. The build was configured with RUSTFLAGS: -D warnings, which treats all warnings as errors. Three specific issues were found: - Unused import ydotool_daemon_socket at crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:3:53 - Unused import InjectionContext at crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:5:37 - Unused variable harness at crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:107:9 These unused imports and variables need to be removed or prefixed with underscore (e.g., _harness) to indicate they are intentionally unused.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 1891: �[36;1m echo "::warning::cargo fmt detected formatting differences. Please run 'cargo fmt --all' locally before committing."�[0m 1892: �[36;1mfi�[0m 1893: �[36;1mexit 0�[0m 1894: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 1895: env: 1896: CARGO_TERM_COLOR: always 1897: WHISPER_MODEL_SIZE: tiny 1898: MIN_FREE_DISK_GB: 10 1899: MAX_LOAD_AVERAGE: 5 1900: CARGO_INCREMENTAL: 0 1901: CARGO_PROFILE_DEV_DEBUG: 0 1902: RUST_BACKTRACE: short 1903: RUSTFLAGS: -D warnings 1904: CARGO_UNSTABLE_SPARSE_REGISTRY: true 1905: CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse 1906: CACHE_ON_FAILURE: true 1907: ##[endgroup] ... 1953: ##[warning]cargo fmt detected formatting differences. Please run 'cargo fmt --all' locally before committing. 1954: ##[group]Run cargo clippy --all-targets --locked 1955: �[36;1mcargo clippy --all-targets --locked�[0m 1956: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 1957: env: 1958: CARGO_TERM_COLOR: always 1959: WHISPER_MODEL_SIZE: tiny 1960: MIN_FREE_DISK_GB: 10 1961: MAX_LOAD_AVERAGE: 5 1962: CARGO_INCREMENTAL: 0 1963: CARGO_PROFILE_DEV_DEBUG: 0 1964: RUST_BACKTRACE: short 1965: RUSTFLAGS: -D warnings 1966: CARGO_UNSTABLE_SPARSE_REGISTRY: true 1967: CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse 1968: CACHE_ON_FAILURE: true 1969: ##[endgroup] ... 2035: �[1m�[92m Compiling�[0m link-cplusplus v1.0.12 2036: �[1m�[92m Checking�[0m atspi-common v0.13.0 2037: �[1m�[92m Compiling�[0m semver v1.0.27 2038: �[1m�[92m Compiling�[0m cxxbridge-flags v1.0.192 2039: �[1m�[92m Compiling�[0m cxx v1.0.192 2040: �[1m�[92m Compiling�[0m qt-build-utils v0.8.0 2041: �[1m�[92m Checking�[0m ort v2.0.0-rc.10 2042: �[1m�[92m Checking�[0m voice_activity_detector v0.2.1 2043: �[1m�[92m Checking�[0m atspi-proxies v0.13.0 2044: �[1m�[92m Compiling�[0m cxx-qt-gen v0.8.0 2045: �[1m�[92m Checking�[0m coldvox-vad-silero v0.1.0 (/home/runner/work/ColdVox/ColdVox/crates/coldvox-vad-silero) 2046: �[1m�[92m Compiling�[0m cxxbridge-macro v1.0.192 2047: �[1m�[92m Checking�[0m atspi-connection v0.13.0 2048: �[1m�[92m Checking�[0m atspi v0.29.0 2049: �[1m�[92m Compiling�[0m cxx-qt-build v0.8.0 2050: �[1m�[91merror�[0m�[1m: unused import: `ydotool_daemon_socket`�[0m 2051: ##[error] �[1m�[94m--> �[0mcrates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:3:53 2052: �[1m�[94m|�[0m 2053: �[1m�[94m3�[0m �[1m�[94m|�[0m candidate_socket_paths, locate_existing_socket, ydotool_daemon_socket, YdotoolInjector, 2054: �[1m�[94m|�[0m �[1m�[91m^^^^^^^^^^^^^^^^^^^^^�[0m 2055: �[1m�[94m|�[0m 2056: �[1m�[94m= �[0m�[1mnote�[0m: `-D unused-imports` implied by `-D warnings` 2057: �[1m�[94m= �[0m�[1mhelp�[0m: to override `-D warnings` add `#[allow(unused_imports)]` 2058: �[1m�[91merror�[0m�[1m: unused import: `InjectionContext`�[0m 2059: ##[error] �[1m�[94m--> �[0mcrates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:5:37 2060: �[1m�[94m|�[0m 2061: �[1m�[94m5�[0m �[1m�[94m|�[0m use crate::types::{InjectionConfig, InjectionContext}; 2062: �[1m�[94m|�[0m �[1m�[91m^^^^^^^^^^^^^^^^�[0m 2063: �[1m�[92m Compiling�[0m coldvox-gui v0.1.0 (/home/runner/work/ColdVox/ColdVox/crates/coldvox-gui) 2064: �[1m�[91merror�[0m�[1m: unused variable: `harness`�[0m 2065: ##[error] �[1m�[94m--> �[0mcrates/coldvox-text-injection/src/tests/test_ydotool_injector.rs:107:9 2066: �[1m�[94m|�[0m 2067: �[1m�[94m107�[0m �[1m�[94m|�[0m let harness = TestHarness::new().unwrap(); 2068: �[1m�[94m|�[0m �[1m�[91m^^^^^^^�[0m �[1m�[91mhelp: if this is intentional, prefix it with an underscore: `_harness`�[0m 2069: �[1m�[94m|�[0m 2070: �[1m�[94m= �[0m�[1mnote�[0m: `-D unused-variables` implied by `-D warnings` 2071: �[1m�[94m= �[0m�[1mhelp�[0m: to override `-D warnings` add `#[allow(unused_variables)]` 2072: �[1m�[91merror�[0m: could not compile `coldvox-text-injection` (lib test) due to 3 previous errors 2073: �[1m�[33mwarning�[0m: build failed, waiting for other jobs to finish... 2074: ##[error]Process completed with exit code 101. 2075: ##[group]Run actions/upload-artifact@v6 ... 2082: if-no-files-found: warn 2083: compression-level: 6 2084: overwrite: false 2085: include-hidden-files: false 2086: env: 2087: CARGO_TERM_COLOR: always 2088: WHISPER_MODEL_SIZE: tiny 2089: MIN_FREE_DISK_GB: 10 2090: MAX_LOAD_AVERAGE: 5 2091: CARGO_INCREMENTAL: 0 2092: CARGO_PROFILE_DEV_DEBUG: 0 2093: RUST_BACKTRACE: short 2094: RUSTFLAGS: -D warnings 2095: CARGO_UNSTABLE_SPARSE_REGISTRY: true 2096: CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse 2097: CACHE_ON_FAILURE: true 2098: ##[endgroup]

[kiloconnect]

✅ Previous Issues Addressed

The following issues from my previous review have been resolved:

• Unused imports: Fixed - removed ydotool_daemon_socket and InjectionContext imports
• Unused variable: Fixed - harness variable is now properly used in tests
• Leftover unused code: Fixed - line 137 now uses let _ = to indicate intentional non-use

Minor remaining note:

• use crate::TextInjector; import appears unused in the current test file

Recommendation: PR is ready to merge - all critical issues resolved

Review Details (3 files)

Files:

• crates/coldvox-text-injection/src/tests/test_ydotool_injector.rs (new test file)
• crates/coldvox-text-injection/src/tests/mod.rs (module registration)
• crates/coldvox-text-injection/src/ydotool_injector.rs (made functions public for testing)

Changes reviewed:

• +216 additions, -3 deletions
• Added comprehensive unit tests for ydotool injector
• Implemented TestHarness for isolated test environments
• Tests cover socket discovery, binary permissions, and text injection

Quality checks:

• ✅ Tests are well-structured with proper isolation
• ✅ Environment setup/teardown handled correctly
• ✅ Mock infrastructure properly implemented
• ✅ Good test coverage for different scenarios
• ⚠️ Minor: TextInjector import unused (low priority)