fix(test): accept both Ask and Allow verdicts in rewrite tests by guyoron1 · Pull Request #3147 · rtk-ai/rtk

guyoron1 · 2026-07-22T12:26:01Z

Heyaaa! : )

I've been working with RTK for a while and noticed two tests in rewrite_cmd.rs kept failing on my machine but passed on fresh checkouts. Tracked it down to my local .claude/settings.local.json — having Bash(git *) allow rules makes check_command("git status") return Allow instead of Default, breaking the exact Ask(_) assertions.

Fix: accept both Ask(...) and Allow(...) since the real intent is "command was rewritten, not Passthrough."

Closes #3146

Test plan

Tests pass WITH settings.local.json containing Bash(git *) allow rules
Tests pass WITHOUT settings.local.json (clean machine)
Full test suite passes (cargo test --all)
cargo fmt --all && cargo clippy --all-targets clean

KuSh · 2026-08-14T22:41:52Z

Hi @guyoron1, and thanks for the PR. Unfortunately this isn’t the right fix. Tests should be insulated from the local machine’s configuration not accommodate it. That could be a great contribution if you'd like to work on it (at least those that affect you at first)

guyoron1 · 2026-08-15T05:03:11Z

You're right — accommodating the local config was the wrong direction. Reworked to insulate instead.

evaluate() called check_command(), which reads the machine's settings files, so the test outcome tracked whatever the developer happened to have configured. It now has the same verdict-injection seam permissions.rs already exposes via check_command_with_rules: evaluate_with_verdict() holds the decision logic and takes the verdict as a parameter, evaluate() stays the thin wrapper that looks it up. The tests pin PermissionVerdict::Default and read no settings at all, so the strict Ask(_) assertions are back.

Worth noting the four Passthrough tests in that module had the same latent problem in the other direction — a Bash(git *) deny rule would have turned them into Deny. They're pinned now too, and I added Allow/Deny cases so the verdict mapping stays covered in both directions rather than losing coverage.

Verified by running the test binary against HOME directories containing deny, ask, allow, and no rules — 8/8 in all four, plus with this repo's own settings.local.json in place. Full suite: 2662 passed, fmt/clippy clean.

CLAassistant · 2026-08-15T05:34:09Z

All committers have signed the CLA.

The unattestable_passthrough tests called evaluate(), which calls check_command() and reads the developer's Claude Code settings files (.claude/settings.local.json, ~/.claude/settings.json). A local `Bash(git *)` allow rule turned the expected Ask into Allow, so the two rewrite assertions failed on that machine and passed everywhere else; deny rules would likewise have broken the four Passthrough assertions. Give evaluate() the same verdict-injection seam that permissions.rs already exposes via check_command_with_rules: evaluate_with_verdict() holds the decision logic and takes the verdict as a parameter, while evaluate() stays the thin wrapper that looks it up. The tests pin PermissionVerdict::Default, so they read no settings at all and assert the exact outcome again rather than accepting either verdict. Added coverage for the Allow and Deny verdicts so the mapping stays tested in both directions without depending on host configuration. Verified by running the test binary with HOME pointed at settings files containing deny, ask, allow, and no rules: 8/8 pass in all four, and with the repo's own settings.local.json in place. Closes rtk-ai#3146

KuSh

LGTM thanks!

Inline evaluate_with_verdict() calls directly in unattestable_passthrough tests instead of through a locally-named eval() alias, and move the two verdict-to-outcome tests (allow/deny) up into the parent tests module since they aren't about unattestable-construct passthrough. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Conflict resolutions: - src/core/stream.rs: kept the fork's CappedCapture (head+tail 10 MiB cap, 9d8a823) and adopted upstream's read_lines_lossy at all six call sites. The two are orthogonal: upstream fixes lines().map_while(Result::ok) dropping every line after the first invalid-UTF-8 one, which on the CaptureOnly path silently truncated whole mvn builds. - src/cmds/system/find_cmd.rs: took upstream wholesale. Its grammar dispatch (rtk-ai#3603) supersedes the fork's has_unsupported_find_flags fallback (0b8d5db) -- -exec/-delete/-printf go verbatim, -not/-size compress via real find, both with never_worse and exit-code propagation. - src/main.rs: restored upstream's Find arm; run_from_args is Result<()> again and exits with the child's code from inside find_cmd. - Cargo.lock: regenerated on top of upstream's (fork dev-deps filetime, insta preserved). Gate: fmt clean, clippy --all-targets clean, cargo test --all 2896 passed. Failures drop 24 -> 22; the two that vanish are the local-settings-dependent rewrite tests upstream fixed in rtk-ai#3147. No new failures.

rtk 0.46.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## [0.46.0](rtk-ai/rtk@v0.45.0...v0.46.0) (2026-08-26) ### Features - find: dispatch on find's grammar; compress find output for unmodeled predicates ([#3603](rtk-ai/rtk#3603)) - find: tee tail hint when rtk imposes the result cap ([#3603](rtk-ai/rtk#3603)) ### Bug Fixes - find: never-worse guard, recovery hint, and dispatch on find's grammar ([#3603](rtk-ai/rtk#3603)) - git: don't misdetect a value-taking option's argument as a patch flag ([#3575](rtk-ai/rtk#3575)) - cicd: stop benchmark.sh deleting the tracked scripts/benchmark harness ([#3595](rtk-ai/rtk#3595)) - tee: hash long recovery-file slugs to prevent collisions and shorten hints ([#3266](rtk-ai/rtk#3266)) - benchmark: avoid negative curl/cargo cases that fail the benchmark job ([#3430](rtk-ai/rtk#3430)) - test: accept both Ask and Allow verdicts in rewrite tests ([#3147](rtk-ai/rtk#3147)) — Closes [#3146](rtk-ai/rtk#3146) - core: decode process output using Windows console code page ([#2717](rtk-ai/rtk#2717)) — Closes [#2452](rtk-ai/rtk#2452) - git: preserve patch output from log commands ([#2951](rtk-ai/rtk#2951)) — Closes [#2944](rtk-ai/rtk#2944) - discover: sanitize drive-letter colon so Windows discover finds sessions ([#2952](rtk-ai/rtk#2952)) — Closes [#2919](rtk-ai/rtk#2919) - stream: decode lossily instead of dropping lines on invalid UTF-8 ([#2997](rtk-ai/rtk#2997)) — Closes [#2994](rtk-ai/rtk#2994) ### Other - test(find): use the platform temp dir instead of /tmp ([#3717](https://github.com/rtk-ai/rtk/pull/3717))</pre> <p>View the full release notes at <a href="https://github.com/rtk-ai/rtk/releases/tag/v0.46.0">https://github.com/rtk-ai/rtk/releases/tag/v0.46.0</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!17826

guyoron1 mentioned this pull request Aug 14, 2026

fix(core): decode process output using Windows console code page #2717

Merged

3 of 4 tasks

guyoron1 force-pushed the fix/rewrite-test-env-sensitivity branch from 891b8ef to 1191128 Compare August 14, 2026 19:01

guyoron1 force-pushed the fix/rewrite-test-env-sensitivity branch from 1191128 to 3dc85a2 Compare August 15, 2026 05:03

guyoron1 force-pushed the fix/rewrite-test-env-sensitivity branch from 3dc85a2 to f0cd98f Compare August 15, 2026 05:34

guyoron1 force-pushed the fix/rewrite-test-env-sensitivity branch from f0cd98f to 0eec668 Compare August 15, 2026 07:00

guyoron1 force-pushed the fix/rewrite-test-env-sensitivity branch from 0eec668 to 668f449 Compare August 16, 2026 10:34

KuSh approved these changes Aug 16, 2026

View reviewed changes

KuSh merged commit 32c83cc into rtk-ai:develop Aug 16, 2026
10 of 11 checks passed

rtk-release-bot Bot mentioned this pull request Aug 16, 2026

Next Release #3544

Merged

aeppling mentioned this pull request Aug 26, 2026

chore(master): release 0.46.0 #3718

Merged

BrewTestBot mentioned this pull request Aug 26, 2026

rtk 0.46.0 Homebrew/homebrew-core#300841

Merged

This was referenced Aug 27, 2026

⬆️ Upstream sync — v0.48.0 AlobarQuest/rtk#3

Merged

[dep-watch] rtk 0.46.0 released (tested: 0.44.1) franklywatson/claude-rig#73

Closed

KuSh mentioned this pull request Sep 6, 2026

refactor(hooks): inject the permission verdict so rewrite_cmd tests don't read ~/.claude/settings.json #2636

Closed

3 tasks done

KuSh mentioned this pull request Sep 26, 2026

bug(tests): rewrite_cmd permission tests read ~/.claude/settings.json — 3/6 fail on a normal dev machine #2640

Closed

Navigation Menu

Repository navigation

fix(test): accept both Ask and Allow verdicts in rewrite tests - #3147

fix(test): accept both Ask and Allow verdicts in rewrite tests#3147
KuSh merged 2 commits into
rtk-ai:developfrom
guyoron1:fix/rewrite-test-env-sensitivity

guyoron1 commented Jul 22, 2026 •

edited

Loading

Uh oh!

KuSh commented Aug 14, 2026

Uh oh!

guyoron1 commented Aug 15, 2026

Uh oh!

CLAassistant commented Aug 15, 2026 •

edited

Loading

Uh oh!

KuSh left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

guyoron1 commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Uh oh!

KuSh commented Aug 14, 2026

Uh oh!

guyoron1 commented Aug 15, 2026

Uh oh!

CLAassistant commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KuSh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guyoron1 commented Jul 22, 2026 •

edited

Loading

CLAassistant commented Aug 15, 2026 •

edited

Loading