test(hook): pin exit codes across rtk rewrite (#3492)

[Protocol-zero-0]

Refs #3492 (also the #2974 / #2735 class).

What

Adds tests/rewrite_exit_code_test.rs, an integration test that treats exit-code propagation through hook rewrites as an invariant, which is what #3492 asks for: every command in a matrix is run twice in a sandboxed git repo, once raw and once as whatever rtk rewrite returns, and the two exit codes must match.

The matrix pairs a rewritten tool (git status/log/diff, ls, grep -r, wc, cat -> rtk read, head -> rtk read --head-lines, find) with a failing or succeeding step, so it covers both the tool's own status and a status flowing through an && / ; chain. The python3 -c / SystemExit cases from #3492 are included when python3 is on PATH.

A second test guards the first: if rule changes ever stop these commands from being rewritten, the exit-code check would silently compare sh with itself and keep passing, so it asserts that at least half of the cases still go through a rewrite.

The sandbox points HOME, XDG_CONFIG_HOME, RTK_DB_PATH and RTK_TEE_DIR at a tempdir, following the existing integration tests, so running it doesn't touch the developer's own config or tracking DB.

Does it catch a regression?

Checked by temporarily changing the final std::process::exit(code) in main.rs to swallow codes above 1. The test fails and names the command:

"ls /nonexistent-rtk-path" exited Some(2), but its rewrite "rtk ls /nonexistent-rtk-path" exited Some(0)

With the code unchanged, both tests pass on develop (b748a5f).

Verification

cargo fmt --all, cargo clippy --all-targets and cargo test --all pass locally (Linux). The file is #![cfg(unix)] like the other process-spawning integration tests.

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved findings affect test determinism and claimed rewrite coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Direct Python cases are skipped and do not test rewrite behavior · New

What changed in this PR

Adds a Unix-only integration test verifying that rtk rewrite preserves command exit codes across tools and shell chains.

Changes:

• Creates an isolated temporary Git sandbox.
• Compares raw and rewritten command statuses.
• Guards against rewrite rules silently disappearing.

File	Summary
tests/​rewrite_exit_code_test.rs	Adds exit-code propagation coverage. Five unresolved moderate findings concern path control, skipped Python and semicolon cases, and incomplete environment isolation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[KuSh]

Round 1 — CHANGES REQUESTED

Claim: an integration test that pins rtk rewrite's exit-code faithfulness: each command in a matrix exits the same raw and as its rewrite, for a rewritten tool's own status and for a status flowing through && / ; lists.
Scope: accept. A regression test for the #3492 / #2974 class is worth merging as stated.
Ran: rtk rewrite then sh -c on the merge of 62becfe onto develop e12ca86. I ran the 16 PR cases plus 12 candidate rows, comparing raw and rewritten exit codes. Invariants: under the hook (rtk rewrite is the hooks' own call; every rewrite answers 3, ask, in the sandbox) and every decision (6 product mutations). Savings: n/a, test-only.
Pins: 6 mutations of the product, 4 survive the PR's tests (git failures swallowed, && printed as ;, || printed as &&, four tools dropping out of the rewrite)
CI @62becfeb: 3 failing: test (ubuntu-latest), test (macos-latest), test (windows-latest), all failing core::test_isolation::tests_that_spawn_rtk_isolate_their_data

Blocks merge (3)

1. tests/rewrite_exit_code_test.rs:59: the sandbox builds its own isolation instead of using tests/common. This affects all three spawn sites: the setup git, Sandbox::command and Sandbox::rewrite.
   • CI red. Since develop e0d99ab5 (2026-09-30, after this branch's base b748a5f), a tests/ file may not name CARGO_BIN_EXE_rtk. Merged onto develop, tests_that_spawn_rtk_isolate_their_data fails with tests/rewrite_exit_code_test.rs: CARGO_BIN_EXE_rtk. That is the red on all three test jobs, and it stops the run before this file's tests execute.
   • Writes to the developer's repository. The children inherit the developer's environment. Git exports GIT_INDEX_FILE to a pre-commit hook, as an absolute path to .git/index.lock under git commit -a. With it exported, Sandbox::new's git add a.txt staged a.txt into that index: a probe repository's index went from keep.txt to a.txt + keep.txt.
   • Fails depending on the machine. A developer who exports CLAUDE_CONFIG_DIR with a deny rule such as Bash(ls:*) gets a red most_cases_actually_go_through_a_rewrite, because rtk rewrite exits 2. In the same run, rewritten_commands_keep_the_raw_exit_code passes while silently skipping every ls case.
   • A fix must build every child through common::rtk_command(), common::isolate_rtk(&mut cmd) and common::temp_git_repo() / common::isolate_git(). It must take rtk's directory for PATH from common::rtk_command().get_program(), as tests/hook_decision_protocol_test.rs:156 does.
   • Rows: the guard is green on the merge; the probe index is unchanged with GIT_INDEX_FILE and GIT_DIR exported; the test passes with that CLAUDE_CONFIG_DIR exported.
2. tests/rewrite_exit_code_test.rs:112: unpinned: no case makes a rewritten tool fail where its status decides the result. No git command fails (git diff --exit-code HEAD runs on a clean tree, so it exits 0). No failing tool heads an && list, and there is no ||.

   • Evidence: three mutations pass 2/2. main.rs exiting 0 for every rtk git failure; rtk rewrite printing && as ;; and printing || as &&.

   • The test must run and match these rows:

     Command	Exit
     git log --no-such-flag	128
     git diff --exit-code with a tracked file changed	1
     git show no-such-rev && sh -c 'exit 9'	128
     ls /nonexistent-rtk-path && sh -c 'exit 13'	2 on Linux
     ls /nonexistent-rtk-path || sh -c 'exit 13'	13
     ls a.txt || sh -c 'exit 13'	0

   • Each of the three mutations must turn it red. All six rows rewrite and match on develop today, so they add coverage, not new failures.

3. tests/rewrite_exit_code_test.rs:174: cases skip silently.
   • true; sh -c 'exit 12' and sh -c 'exit 5' are never rewritten (rtk rewrite exits 1). The loop continues past them, so they test nothing, including the ; list the description names.
   • The guard's rewritten.len() * 2 >= CASES.len() lets 7 of the 14 cases drop out unnoticed. Evidence: rtk rewrite deferring on cat/head/wc/find, plus rtk read/wc/find exiting 0, passes 2/2, with 8 of 14 cases still rewritten.
   • The test must assert each case's rewrite one by one, as it already does for PYTHON_CASES, and cover ; with a rewritten head, e.g. git status; sh -c 'exit 12' (12). That mutation must turn it red.

Checked and correct (no need to re-verify)

• On the merged binary, every PR case that rewrites (14 of 16) exits the same raw and rewritten. That includes both compound Python cases.
• The description's own regression check reproduces. With main.rs swallowing codes above 1, the test fails naming ls /nonexistent-rtk-path (2 vs 0). With every failure swallowed, it names 5 cases.
• The Copilot thread on bare python3 -c is fixed in 62becfe: the bare cases are gone, their passthrough is documented, and each compound Python case is asserted rewritten.
• With XDG_DATA_HOME exported, the test writes nothing there.

Your questions

None asked.

Next

Merge develop and move the sandbox onto tests/common (1). Add the failing-head and || rows (2). Assert each case's rewrite instead of a 50% threshold (3). After that, this merges once CI is green.
Threads: 3 open (the blockers above), 1 resolved this round (Copilot's Python thread, fixed by 62becfe).

[KuSh]

Round 2 — APPROVED

Claim: an integration test that pins rtk rewrite's exit-code faithfulness: each command in a matrix exits the same raw and as its rewrite, for a rewritten tool's own status and for a status flowing through && / || / ; lists.
Scope: accept (round 1, frozen).
Ran: rtk rewrite then sh -c on 7087fe9 merged onto develop 5eb0509, all 21 cases with their pinned raw codes. Hostile environments, each a separate run: GIT_INDEX_FILE + GIT_DIR at a probe repository; CLAUDE_CONFIG_DIR denying Bash(ls:*) and Bash(git:*); LC_ALL=de_DE.UTF-8; RTK_NO_TOML=1 RTK_TEE=0; TMPDIR inside a git repository; busybox ls/find first on PATH; a python3 shim that resolves through the caller's HOME. Invariants: under the hook, every decision. Savings: n/a, test-only.
Pins: 7 product mutations. The head catches 6. The seventh, rtk git diff always exiting at least 1, is closed by the fixup below.
CI @7087fe9f: 12/12 green, and the new test ran on macOS. @b7d9f5d00: 15/15 green.

Blocks merge (3, frozen at round 1): all fixed in 7087fe9

1. Isolation. All 6 test_isolation checks are green on the merge. The probe repository's index is byte-identical after a run with GIT_INDEX_FILE and GIT_DIR exported, and the test passes with deny rules in CLAUDE_CONFIG_DIR.
2. Failing heads. Swallowing git failures, printing && as ; and printing || as && each turn the test red now.
3. Silent skips. Every case must rewrite. Dropping cat/head/wc/find from the rewrite fails both tests.

Fixup b7d9f5d: what it changes

• The ls missing-operand code is probed once in the sandbox and must be 1 or 2. It used to be keyed on cfg!(macos). With busybox ls first on PATH (exit 1), the head failed with raw fixture exit changed: ls /nonexistent-rtk-path. A macOS shell with GNU coreutils first on PATH is the mirror case.
• The python3 probe runs in the sandbox's own environment. A version-manager shim that resolves through the caller's HOME (the asdf/mise pattern) passed the old probe, then exited 126 inside the sandbox and failed git status && python3 ….
• A clean-diff row: git diff --cached --exit-code && sh -c 'exit 7' (7). My round-1 row made the fixture dirty, which left no case where git diff --exit-code succeeds, so rtk git diff always exiting at least 1 passed the suite. This row restores that coverage.
• The rewrite-guard test iterates both tables directly. It now checks the Python rows' rewrites even where python3 cannot run, and a bad ls probe no longer fails it.
• .expect replaces .unwrap() for the binary's directory, and three comments now describe the fixture and the table as they are.

Checked and correct (no need to re-verify)

• On the merge, every row's raw code matches its pin, and its rewrite exits the same.
• The full suite on the merge is green apart from one develop test that needs a local Go toolchain and fails the same way without this PR.

Your questions

None asked.

Next

Nothing: fixup b7d9f5d pushed and code-reviewed. Approving on green CI.
Threads: 0 open. The 3 round-1 threads are resolved, and each is fixed in 7087fe9.