Repository navigation
fix(git): make the blob recovery hint work under RTK's own hook - #4118
Conversation
📊 Automated PR Analysis
SummaryFixes a regression where the blob recovery hint printed for windowed Review Checklist
Analyzed automatically by wshm · This is an automated analysis, not a human review. |
pszymkowiak
left a comment
There was a problem hiding this comment.
Reviewed by building the branch and following the hint the way an agent would, with the hook simulated through rtk rewrite.
Verified
- develop: the hint names bare
git show 'HEAD:src/core/utils.rs' | tail -n +250;rtk rewriteturns it intortk git show … | tail -n +250(exit 3); following it yields 1 line (the hint itself); head + rest ≠ native (sha256mismatch, 8189+84 bytes vs 65645). - this PR: hint names
rtk proxy git show '…' | tail -n +250;rtk rewriteleaves it alone (exit 1,rtk-prefixed segment returned unchanged atregistry.rs:1691,tail -n +Ncannot matchTAIL_N_SPACE); following it yields 1593 lines; head + rest == native byte for byte (8189 + 57456 = 65645, same sha256). - Adjacent shapes: path with a space,
<sha>:<path>,rtk git -C <abs> show …from outside the repo (hint carries'-C' '<path>'through proxy), small blob → no hint. All correct. compact_blob_showhas a single production caller (git_cmd.rs:606); no stale copy of the old hint string remains insrc/,tests/ordocs/.
One precision note, no code change needed: windowed_blob_hint_command_returns_the_rest executes the hint's rtk proxy git show argv and applies the tail in-process, but does not run the hook; the hook-survival guarantee is the unit test test_compact_blob_show_hint_survives_rtks_own_hook calling the real rewrite_command. Together they cover both halves; the doc comment on the integration test could say so.
Approving. CI green on all three OSes.
A windowed `git show <rev>:<path>` points at the rest of the file with `| tail -n +N`. The hook rewrites the bare `git show` in that hint back into `rtk git show`, which windows the dump a second time, so `tail` had only the hint itself left to print: 1 line where the reader asked for 1434. Name the command the hook leaves alone, as the `diff` size hints already do. The differential test now runs the command the hint names instead of simulating it, so a hint that cannot return the blob fails CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
468b0b8 to
f9415c5
Compare
Fixes one regression from the release review on #3979, introduced by #3265.
Part of a set of six, one per originating PR: #3681, #3552, #3265 (this), #3772, #3857, #3941.
The regression
A windowed
git show <rev>:<path>prints a head and points at the rest of the file:Under RTK's own hook, the bare
git showin that hint is rewritten back intortk git show, which windows the dump a second time — sotail -n +249had nothing left to print but the hint itself.The fix
Name the command the hook leaves alone.
rtk proxyis the documented escape hatch and thediffsize hints insrc/cmds/git/diff_cmd.rsalready use it the same way:248 shown lines + 1434 recovered = 1682 = the native
git showtotal, byte for byte.Verification
tests/git_show_blob_differential_test.rsgainedwindowed_blob_hint_command_returns_the_rest, which runs the command the hint names instead of simulating it — it parses the hint's own argv, executes it, and asserts head + recovered output reconstructs the blob byte for byte. Against the old hint it fails withthe hint returned 1 line(s) -- it must return the rest of the blob, not just itself, so it is a real guard for exactly this regression; the previous tests reconstructed the tail with native git and could never have seen it.A unit test also asserts the emitted hint is left untouched by
discover::registry::rewrite_command, which is the function all three hook front-ends funnel through.docs/usage/FEATURES.mdquoted the old hint string literally; it now shows thertk proxyform and notes that a baregit showunder the hook re-windows.cargo fmt --all,cargo clippy --all-targetsandcargo test --allare green, rebased on currentdevelop.🤖 Generated with Claude Code