fix(main): kill zombie processes + path for rtk md + missing intergrations#978
fix(main): kill zombie processes + path for rtk md + missing intergrations#978
Conversation
|
Documentation cicd does not pass but this may be wrong since i have already commented the edge case and issue in the code, which should match with incoming new coding practice |
Also fixes flaky test_rewrite_rtk_disabled_warns_on_stderr: the subprocess part relied on target/debug/rtk being current, but cargo test doesn't rebuild the standalone binary. Added mtime guard -> skips when binary is older than test executable.
pszymkowiak
left a comment
There was a problem hiding this comment.
Thanks @aeppling — good set of improvements. Build & tests pass (1260 passed, 3 ignored). Reviewed code and ran manual tests on the release binary.
P0 — must fix:
1. ChildGuard calls kill() on already-reaped process
src/main.rs — After the normal child.0.wait() succeeds, the Drop still calls kill() + wait() on the dead process. Harmless on Unix today but incorrect semantics + theoretical PID recycling race. Fix: disarm the guard after successful wait:
struct ChildGuard(Option<std::process::Child>);
impl Drop for ChildGuard {
fn drop(&mut self) {
if let Some(mut child) = self.0.take() {
let _ = child.kill();
let _ = child.wait();
}
}
}
// After wait() succeeds:
let status = child.0.take().unwrap().wait()?;2. rtk read - - silently produces empty output on second stdin read
src/main.rs Read loop — The second run_stdin() call consumes EOF, returning empty output with exit 0. Should warn on stderr:
let mut stdin_seen = false;
for file in &files {
if file == Path::new("-") {
if stdin_seen {
eprintln!("rtk: warning: stdin specified more than once");
}
stdin_seen = true;
// ...
}
}P1 — important:
3. hook_check.rs — other_integration_installed masks Claude Code missing-hook warning
When .claude/ exists but the hook is missing, the presence of Cursor/Codex/OpenCode suppresses the "No hook installed" warning. A Claude Code user with Cursor installed for another project will never see the alert. The early exit at line 29 (no .claude dir) already handles the "user doesn't use Claude Code" case.
4. docs/contributing/ARCHITECTURE.md — broken relative links after move
hooks/README.md (line 38) and CONTRIBUTING.md (line 1034) still reference paths relative to root, but the file is now at docs/contributing/. These should be ../../hooks/README.md and ../../CONTRIBUTING.md.
5. init.rs — misleading confirmation message in global mode
Line 1291 prints @RTK.md reference added but in global mode the actual reference is an absolute path. Should use the rtk_md_ref variable.
6. No tests for multi-file read loop
The main behavioral change (single file to multi-file) has no test coverage for: two valid files concatenated, mix of valid + nonexistent, stdin dedup.
Notes:
ROADMAP.mddeletion: fine, it was obsolete (15 lines, all items completed)filter-workflow.mddeletion: this was the only mermaid diagram of the TOML pipeline — intentional?print!vsprintln!change: verified correct withxxd, no trailing newline regression
Fix P0s and it's ready.
#892
#897
#913
#989
Test plan
cargo fmt --all && cargo clippy --all-targets && cargo testrtk <command>output inspected