Bump edition 2021 -> 2024. rust-version stays at 1.91, already well above
the 1.85 floor the edition needs; docs/guide/resources/troubleshooting.md
still claimed 1.70+, which is where a failed `cargo install --git` lands.
Four things the edition forces:
- std::env::{set_var,remove_var} are unsafe in 2024 with no safe std
replacement. Rather than wrap the test call sites in unsafe -- which the
crate denies and .semgrep.yml flags -- route them through temp-env, a
dev-only dependency whose closure API is safe and which restores the
previous value even when the body panics. The hand-rolled CLAUDE_DIR_LOCK
and PI_DIR_LOCK guards existed only to serialise those mutations and are
now redundant; CWD_LOCK and TEST_ENV_LOCK stay, they order more than the
env var itself.
- unsafe_op_in_unsafe_fn is on by default, so the libc calls in the proxy
signal handler and in stream.rs's relay handler need explicit unsafe
blocks, scoped to the libc calls themselves.
- `gen` is a reserved keyword, so the closure by that name in diff_cmd.rs
becomes make_lines.
- Tightened tail-expression temporary scopes let clippy prove the binding in
setup_test_env is inlinable, so let_and_return now fires there.
if_let_rescope changes when the scrutinee temporary drops in an if let/else.
The two sites in show_claude_config take cargo fix --edition's match rewrite,
which keeps the 2021 drop timing.
rustfmt.toml is kept rather than dropped: cargo fmt passes --edition from
Cargo.toml, but a bare rustfmt invocation has no crate context and falls
back to edition 2015, which cannot parse the let-chains the next commit
introduces. Pinning it there keeps format-on-save and pre-commit hooks in
agreement with CI.
clippy::collapsible_if is allowed crate-wide for now; the follow-up commit
adopts let-chains and removes the allow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every ecosystem mod.rs under src/cmds/ generated its module list with
automod::dir!(). rustfmt only follows literal `mod` items and cannot expand
macros, so `cargo fmt` never reached any of those files -- and neither did
CI's `cargo fmt --all -- --check` gate, which has been silently passing
over half the crate.
Demonstrated by appending `fn __probe( )->u8{let x=1;x}` to a file
under src/cmds/: with automod, `cargo fmt --all -- --check` reports no
diff; with an explicit `pub mod`, it reports the diff and `cargo fmt --all`
fixes it. The preceding commit had to invoke rustfmt directly for the same
reason; from here `cargo fmt` covers the crate on its own.
Listing modules by hand trades one failure mode for another, so build.rs
now guards the new one: automod compiled any stray .rs file, whereas an
explicit list silently drops a file whose `pub mod` line is forgotten --
never compiled, never linted, tests never run, and check-test-presence.sh
still reporting PASS because it only greps the file for #[cfg(test)]. The
build now fails with the missing declaration named.
Declaring the modules explicitly also exposed clippy::module_inception on
cmds::git::git, which clippy skips inside macro-generated code. git.rs is
renamed to git_cmd.rs, which resolves the lint and matches the convention
its siblings already follow -- diff_cmd, gh_cmd, glab_cmd, gt_cmd. Docs
that point at the module are updated; sample command output that merely
shows a "git.rs" path is left alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addresses the inline review on #2717.
decode_process_output
- Decode a line at a time instead of reinterpreting the whole buffer at
the first bad byte. Valid UTF-8 lines keep their bytes; only lines that
fail UTF-8 validation go through the code page, so one stray byte no
longer mangles output that was almost entirely UTF-8. The line is the
unit because a byte run is not one: GB18030's four-byte sequences embed
bytes in the ASCII digit range, so any rule that ends a run below 0x80
splits them. \n cannot appear as a trail byte in any encoding handled
here, and a process does not switch encoding mid-line.
- A code page result is only accepted when it decodes cleanly, so a UTF-8
line with a corrupt byte falls back to lossy UTF-8 rather than mojibake.
- Replace the hand-written code page table with the codepage crate, as
suggested. That also fixes 54936, which was mapped to GBK and now
correctly resolves to gb18030.
- Add oem_cp for the legacy OEM/DOS pages (437, 850, 852, …) that plain
cmd.exe still defaults to in many locales. encoding_rs implements only
WHATWG encodings, so codepage alone returns None for them.
- Fall back to GetACP when GetConsoleOutputCP reports no console, which
is the piped case rtk normally runs in, and warn once instead of
falling back to lossy silently.
- Cache the code page lookup in a OnceLock.
- The mapping and the walk take the code page as a parameter, so they are
compiled and unit-tested on every platform rather than only Windows.
Call sites
- Route the remaining production sites through stream::exec_capture and
exec_capture_stdin rather than decoding at each one, so future callers
inherit decoding. git commit keeps inherited stdin via the _stdin
variant. Test-only sites go back to from_utf8_lossy: they assert on
rtk's own UTF-8 output, where a console code page has no meaning.
- Decode the streamed path (read_lines_lossy) too — the OEM/ANSI lines
its comment describes were still going straight to U+FFFD.
- curl keeps its body on from_utf8_lossy: a response body is a network
payload whose encoding comes from the HTTP charset, not the local
console, and non-UTF-8 bodies already take the binary passthrough for
#1087. Only curl's own stderr is code page decoded.
git commit summary parsing
- parse_commit_output sliced from byte 1, which panics when the first
line starts with a multi-byte character — git prints hook output before
its summary, and a lossily decoded line starts with a multi-byte
U+FFFD. Locate the bracket pair with find instead, so both indices are
character boundaries.
Verified: unit tests for the walk, GBK, gb18030, CP437/850, mixed lines,
truncated input and every byte value; a test pinning that output without
a code page stays byte-identical to from_utf8_lossy; and the Windows-only
lookup cross-compiled for x86_64-pc-windows-msvc.
Review feedback (KuSh): the helper existed but most filter modules still
called String::from_utf8_lossy directly, so non-UTF-8 console output
(e.g. GBK on Chinese-locale Windows) was still mangled for most commands.
Wire decode_process_output through the remaining child-output call sites:
git, go, aws, curl (non-binary paths only — raw binary passthrough for
#1087 is untouched), system read, and discover registry. File-content
decoding (dotnet TRX XML, hook trust snippets) intentionally keeps
from_utf8_lossy since console code pages don't apply there.
On Windows with non-UTF-8 console code pages (e.g., GBK for Chinese
locale), child process output is mis-decoded by String::from_utf8_lossy,
producing mojibake. Add decode_process_output() that detects the console
output code page via GetConsoleOutputCP() and decodes with encoding_rs.
Replaces from_utf8_lossy in the core capture paths (exec_capture,
exec_capture_stdin, TOML filter path, proxy streaming path). Module-
specific call sites left for follow-up.
Fixes#2452
Keep develop's [Unreleased] section on top, followed by master's
released versions (0.34.3, 0.34.2).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop-based ChildGuard doesn't run on signals with panic=abort (release
profile). Register a signal handler that stores the child PID in an
AtomicU32 and kills it on SIGINT/SIGTERM, then re-raises the signal
with default handler for correct exit status propagation.
Fixes orphan processes when rtk proxy is killed by Claude Code, process
managers, or manual kill — the root cause behind issue #897.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Nicolas Le Cam