feat: Windows/PowerShell support, runtime snip check, pipe fix, deduplication#26
feat: Windows/PowerShell support, runtime snip check, pipe fix, deduplication#26qSebastiaNp wants to merge 3 commits into
Conversation
…eduplication - Replace hardcoded UNPROXYABLE_COMMANDS with runtime snip check/run (PR VincentHardouin#14) Requires snip >= 0.16.0. Uses 'snip check --' to determine wrapping eligibility. - Fix pipe handling to wrap ALL pipe segments, not just the first (PR VincentHardouin#14) - Add stripSnipPrefixes to prevent snip/snip/snip duplication (PR VincentHardouin#17) - Add Windows support: use 'where snip' instead of 'which snip' - Add PowerShell support: skip , splatting, cmdlets Verb-Noun (PR VincentHardouin#21) - Add newline splitting for multi-line commands (PR VincentHardouin#21) - Add heredoc safety: don't split heredoc body on newlines (PR VincentHardouin#21) - Use client.app.log instead of console.warn - Add system prompt injection via experimental.chat.system.transform - Add quote-aware pipe splitting to avoid splitting pipes inside quotes - Add error guard: leave command unmodified if shouldWrap throws
Greptile SummaryThis PR expands the snip command wrapper across more shells and command shapes. The main changes are:
|
| Filename | Overview |
|---|---|
| src/index.ts | Adds the main wrapping pipeline, platform checks, pipe splitting, runtime snip checks, and heredoc handling. |
| src/index.test.ts | Adds coverage for wrapping, redirections, pipe handling, PowerShell skips, newline splitting, and simple heredoc bodies. |
Reviews (3): Last reviewed commit: "fix: heredoc operator splitting and full..." | Re-trigger Greptile
| const result = | ||
| w1 !== undefined | ||
| ? await $`snip check -- ${w0} ${w1}`.nothrow().quiet() | ||
| : await $`snip check -- ${w0}`.nothrow().quiet() |
There was a problem hiding this comment.
Raw Partial Check Misclassifies Commands
shouldWrap sends only the first two whitespace-split words to snip check and interpolates them as raw shell text. Commands whose eligibility depends on later flags can be checked against the wrong input, and quoted words like echo "hello world" can make the check command syntactically invalid, causing the command to run unwrapped.
- &> redirection: negative lookahead prevents splitting 'cmd &> out.log' - | & pipe-stderr: splitByPipe preserves operators ( | , | &, ||) - heredoc body pipes: skip pipe splitting when heredoc detected - cmdlet regex: only check on Windows to avoid matching apt-get etc. - shouldWrap: only pass first word to snip check
- Heredoc: find delimiter boundaries, only split operators OUTSIDE heredoc body (fixes 'cat <<EOF\na && b\nEOF' being broken) - shouldWrap: pass full command to snip check instead of just first word (fixes 'sudo apt-get', 'time go test', 'env FOO=bar' checks)
| if (heredocMatch) { | ||
| const delimiter = heredocMatch[1] | ||
| const bodyStart = heredocMatch.index! + heredocMatch[0].length | ||
| const bodyEnd = command.indexOf(delimiter, bodyStart) |
There was a problem hiding this comment.
indexOf(delimiter, bodyStart) treats the first matching text as the heredoc terminator, even when it appears inside the body. A command such as cat <<EOF\nEOF marker\necho hi\nEOF is valid shell input, but this code splits at the EOF in EOF marker. The remaining body then goes through normal operator splitting, so body lines can be rewritten with snip run -- and change what the shell receives. Match the delimiter only when it appears as a real heredoc terminator line, including the tab-prefixed form for <<-.
Summary
Combines improvements from multiple community contributions:
UNPROXYABLE_COMMANDSwith runtimesnip checkcalls. Requires snip >= 0.16.0.snip snip snip go testduplication.where snipinstead ofwhich snip.$env, splatting, cmdlets, call operator.shouldWrapthrows.experimental.chat.system.transform.client.app.loginstead ofconsole.warn.Breaking Changes
Minimum snip version is now >= 0.16.0 (
snip run --syntax).Testing
60 tests passing.