Skip to content

fix: use snip check/run subcommands, fix pipe handling, fix multi-word args for snip check #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lenucksi wants to merge 10 commits into
base: main
Choose a base branch
from
+284 −137
Open

fix: use snip check/run subcommands, fix pipe handling, fix multi-word args for snip check #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
eed7f17
feat: use snip check/run subcommands instead of hardcoded unproxyable…
lenucksi May 18, 2026
2aa8086
fix: split cmd into separate args for snip check
lenucksi May 18, 2026
2ea4f7a
fix: handle all pipe segments, not just the first
lenucksi May 18, 2026
d27373f
docs: add pipe handling documentation
lenucksi May 18, 2026
9642c59
feat: prevent double snip prefix in &&/| compound commands
lenucksi May 18, 2026
882fa4b
refactor: apply PR feedback - simplify tests, add snip version check,…
lenucksi May 18, 2026
dad0801
fix: add .nothrow() to hasSnipSubcommands to avoid false-positive on …
lenucksi May 18, 2026
a421df4
fix: use client.app.log for type-safe logging, add dev commands to RE…
lenucksi May 19, 2026
952faf4
fix: pass first two words as separate raw args to snip check (avoid a…
lenucksi May 20, 2026
bdb4410
feat: inject usage instructions into system prompt via experimental.c…
lenucksi May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ The plugin uses the `tool.execute.before` hook to prefix all commands with `snip

## Development

```bash
npm ci # install dependencies (matches CI)
npm run typecheck # TypeScript type checking (CI step 1)
npm test # run tests with vitest (CI step 2)
```

This package uses [semantic-release](https://semantic-release.gitbook.io/) for automated releases. Commit messages should follow the [Conventional Commits](https://www.conventionalcommits.org/) format:

- `fix:` → patch release
Expand Down
253 changes: 167 additions & 86 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, it, expect, beforeEach } from "vitest"
import { toolExecuteBefore } from "./index"
import { createToolExecuteBefore, hasSnipSubcommands } from "./index"

const mockedWrap = async () => true

describe("toolExecuteBefore", () => {
let mockInput: { tool: string; sessionID: string; callID: string }
Expand All @@ -10,190 +12,269 @@ describe("toolExecuteBefore", () => {
mockOutput = { args: { command: "" } }
})

it("should prefix simple command with snip", async () => {
it("should prefix simple command with snip run --", async () => {
mockOutput.args.command = "go test ./..."
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip go test ./...")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- go test ./...")
})

it("should handle command with one env var prefix", async () => {
mockOutput.args.command = "CGO_ENABLED=0 go test ./..."
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("CGO_ENABLED=0 snip go test ./...")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("CGO_ENABLED=0 snip run -- go test ./...")
})

it("should handle command with multiple env var prefixes", async () => {
mockOutput.args.command = "CGO_ENABLED=0 GOOS=linux go test ./..."
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("CGO_ENABLED=0 GOOS=linux snip go test ./...")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("CGO_ENABLED=0 GOOS=linux snip run -- go test ./...")
})

it("should handle command with &&", async () => {
mockOutput.args.command = "go test && go build"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip go test && snip go build")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- go test && snip run -- go build")
})

it("should handle command with |", async () => {
mockOutput.args.command = "git log | head"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip git log | head")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- git log | snip run -- head")
})
Comment on lines 39 to 43
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale pipe test expectations cause multiple failures

With the new multi-segment pipe processing, every pipe segment is independently evaluated by shouldWrap. Because WRAPPED_COMMANDS includes "head", "grep", "jq", "cat", and "echo" (all matched by baseCmd), every segment of a pipeline gets wrapped. However, the expected values throughout this section were copied from the old single-segment tests and still show only the first segment wrapped.

Concrete failures:

  • git log | head → actual snip run -- git log | snip run -- head, expected snip run -- git log | head
  • cat file.json | jq '.content | .text' → actual snip run -- cat file.json | snip run -- jq '.content | .text', expected snip run -- cat file.json | jq '.content | .text'
  • find / -name "*.log" 2>&1 | grep error → both find and grep get wrapped, expected only find
  • echo "hello | world" | cat → both echo and cat wrapped, expected only echo

The WRAPPED_COMMANDS set also contains full-pipeline strings like "cat file.json | jq '.content | .text'" that can never match after pipe splitting — those entries are dead weight. Either remove head, grep, cat, jq, echo from WRAPPED_COMMANDS so the mock simulates a realistic snip check (only git, go, etc. have filters), or update the expected values to reflect both segments being wrapped.

Copy link
Copy Markdown
Author

@lenucksi lenucksi May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — WRAPPED_COMMANDS/defaultShouldWrap removed in favor of mockedWrap = async () => true. All pipe expectations updated so every segment is wrapped. Both git log | head and cat file.json | jq ... expectations now correctly show both segments prefixed.


it("should handle command with ;", async () => {
mockOutput.args.command = "go test; go build"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip go test; snip go build")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- go test; snip run -- go build")
})

it("should handle command with ||", async () => {
mockOutput.args.command = "test -f foo.txt || echo missing"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip test -f foo.txt || snip echo missing")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- test -f foo.txt || snip run -- echo missing")
})

it("should handle command with &", async () => {
mockOutput.args.command = "sleep 1 & sleep 2 &"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip sleep 1 & snip sleep 2 &")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- sleep 1 & snip run -- sleep 2 &")
})

it("should handle mixed operators", async () => {
mockOutput.args.command = "go test && go build; go run"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip go test && snip go build; snip go run")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- go test && snip run -- go build; snip run -- go run")
})

it("should handle env vars with operators", async () => {
mockOutput.args.command = "FOO=bar go test && go build"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("FOO=bar snip go test && snip go build")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("FOO=bar snip run -- go test && snip run -- go build")
})

it("should not double prefix already prefixed command", async () => {
mockOutput.args.command = "snip go test"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip go test")
mockOutput.args.command = "snip run -- go test"
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- go test")
})

it("should not modify non-bash tool calls", async () => {
mockInput.tool = "read"
mockOutput.args.command = "go test"
await toolExecuteBefore(mockInput, mockOutput)
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("go test")
})

describe("unproxyable shell builtins", () => {
it("should skip cd", async () => {
mockOutput.args.command = "cd /tmp"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp")
})

it("should skip source", async () => {
mockOutput.args.command = "source ~/.bashrc"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("source ~/.bashrc")
})

it("should skip . (dot)", async () => {
mockOutput.args.command = ". ./env.sh"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe(". ./env.sh")
})

it("should skip export", async () => {
mockOutput.args.command = "export FOO=bar"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("export FOO=bar")
describe("subcommand passthrough", () => {
it("should pass each pipe segment to shouldWrap", async () => {
const called: string[] = []
const spy = async (c: string) => { called.push(c); return true }
mockOutput.args.command = "git log | head"
await createToolExecuteBefore(spy)(mockInput, mockOutput)
expect(called).toEqual(["git log", "head"])
})

it("should skip alias", async () => {
mockOutput.args.command = 'alias ll="ls -la"'
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('alias ll="ls -la"')
it("should pass each && segment to shouldWrap", async () => {
const called: string[] = []
const spy = async (c: string) => { called.push(c); return true }
mockOutput.args.command = "cd /tmp && go test"
await createToolExecuteBefore(spy)(mockInput, mockOutput)
expect(called).toEqual(["cd /tmp", "go test"])
})

it("should skip unset", async () => {
mockOutput.args.command = "unset VAR"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("unset VAR")
it("should pass mixed operator segments to shouldWrap", async () => {
const called: string[] = []
const spy = async (c: string) => { called.push(c); return true }
mockOutput.args.command = "go test && go build; go run"
await createToolExecuteBefore(spy)(mockInput, mockOutput)
expect(called).toEqual(["go test", "go build", "go run"])
})

it("should skip export with env var prefix", async () => {
mockOutput.args.command = "CGO_ENABLED=0 export FOO=bar"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("CGO_ENABLED=0 export FOO=bar")
it("should not call shouldWrap for already prefixed command", async () => {
const called: string[] = []
const spy = async (c: string) => { called.push(c); return true }
mockOutput.args.command = "snip run -- go test"
await createToolExecuteBefore(spy)(mockInput, mockOutput)
expect(called).toEqual([])
})

it("should skip cd but snip chained command", async () => {
mockOutput.args.command = "cd /tmp && ls"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp && snip ls")
it("should not call shouldWrap for already prefixed segments in compound command", async () => {
const called: string[] = []
const spy = async (c: string) => { called.push(c); return true }
mockOutput.args.command = "cd /tmp && snip run -- go test"
await createToolExecuteBefore(spy)(mockInput, mockOutput)
expect(called).toEqual(["cd /tmp"])
})
})

describe("redirections with &", () => {
it("should not break 2>&1 redirection", async () => {
mockOutput.args.command = "find / -name \"*.log\" 2>&1"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip find / -name \"*.log\" 2>&1")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- find / -name \"*.log\" 2>&1")
})

it("should not break 1>&2 redirection", async () => {
mockOutput.args.command = "cmd 1>&2"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cmd 1>&2")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cmd 1>&2")
})

it("should handle 2>&1 with pipe", async () => {
mockOutput.args.command = "find / -name \"*.log\" 2>&1 | grep error"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip find / -name \"*.log\" 2>&1 | grep error")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- find / -name \"*.log\" 2>&1 | snip run -- grep error")
})

it("should handle 2>&1 with chained commands", async () => {
mockOutput.args.command = "cmd1 2>&1 && cmd2"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cmd1 2>&1 && snip cmd2")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cmd1 2>&1 && snip run -- cmd2")
})
})

describe("pipe expressions with quotes", () => {
it("should not split pipes inside single quotes", async () => {
mockOutput.args.command = "cat file.json | jq '.content | .text'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat file.json | jq '.content | .text'")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cat file.json | snip run -- jq '.content | .text'")
})

it("should not split pipes inside double quotes", async () => {
mockOutput.args.command = 'cat file.json | jq ".content | .text"'
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('snip cat file.json | jq ".content | .text"')
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('snip run -- cat file.json | snip run -- jq ".content | .text"')
})

it("should handle jq with fromjson", async () => {
mockOutput.args.command = "cat file.json | jq '.content[0].text | fromjson'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat file.json | jq '.content[0].text | fromjson'")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cat file.json | snip run -- jq '.content[0].text | fromjson'")
})

it("should handle multiple pipes in jq", async () => {
mockOutput.args.command = "cat file.json | jq '.a | .b | .c'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat file.json | jq '.a | .b | .c'")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cat file.json | snip run -- jq '.a | .b | .c'")
})

it("should handle pipe with || operator", async () => {
mockOutput.args.command = "cmd1 || cmd2"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cmd1 || snip cmd2")
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cmd1 || snip run -- cmd2")
})

it("should handle mixed quotes and pipes", async () => {
mockOutput.args.command = 'echo "hello | world" | cat'
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('snip echo "hello | world" | cat')
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('snip run -- echo "hello | world" | snip run -- cat')
})
})

describe("error guard", () => {
it("should leave command unmodified when shouldWrap throws", async () => {
const throwWrap = async () => { throw new Error("boom") }
mockOutput.args.command = "go test ./..."
await createToolExecuteBefore(throwWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("go test ./...")
})

it("should leave command unmodified when shouldWrap throws for compound commands", async () => {
const throwWrap = async () => { throw new Error("boom") }
mockOutput.args.command = "go test && go build"
await createToolExecuteBefore(throwWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("go test && go build")
})
})
})

describe("mixed wrapping in compound commands", () => {
it("should wrap only segments that shouldWrap approves", async () => {
const selectiveWrap = async (cmd: string) => !cmd.startsWith("cd ")
mockOutput.args.command = "cd /tmp && go test"
await createToolExecuteBefore(selectiveWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp && snip run -- go test")
})

it("should skip all segments when shouldWrap always returns false", async () => {
const neverWrap = async () => false
mockOutput.args.command = "go test && go build"
await createToolExecuteBefore(neverWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("go test && go build")
})
})

describe("already-prefixed segments in compound commands", () => {
it("should not double-prefix snip run -- in && chain", async () => {
mockOutput.args.command = "cd /tmp && snip run -- go test"
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cd /tmp && snip run -- go test")
})

it("should not double-prefix snip run -- with env vars", async () => {
mockOutput.args.command = "FOO=bar snip run -- go test"
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("FOO=bar snip run -- go test")
})

it("should not double-prefix snip run -- in pipe chain", async () => {
mockOutput.args.command = "cd /tmp && snip run -- go test | ls"
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cd /tmp && snip run -- go test | snip run -- ls")
})

it("should not double-prefix snip run -- in complex chain with 2>&1", async () => {
mockOutput.args.command = "cd /tmp && snip run -- go test 2>&1 | ls"
await createToolExecuteBefore(mockedWrap)(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip run -- cd /tmp && snip run -- go test 2>&1 | snip run -- ls")
})
})
})

describe("hasSnipSubcommands", () => {
it("should return true when snip check succeeds", async () => {
const mock$ = ((...args: any[]) => ({
nothrow: () => ({
quiet: async () => ({ exitCode: 0 })
})
})) as any
expect(await hasSnipSubcommands(mock$)).toBe(true)
})

it("should return true even when snip check exits non-zero (filter exists, cmd has no filter)", async () => {
const mock$ = ((...args: any[]) => ({
nothrow: () => ({
quiet: async () => ({ exitCode: 1 })
})
})) as any
expect(await hasSnipSubcommands(mock$)).toBe(true)
})

it("should return false when snip check subcommand is missing", async () => {
const mock$ = ((...args: any[]) => ({
nothrow: () => ({
quiet: async () => { throw new Error("not found") }
})
})) as any
expect(await hasSnipSubcommands(mock$)).toBe(false)
})
})
Loading