fix(command-semantics): treat linter exit 1 as violations, not an error (#1846)

* fix(command-semantics): treat linter exit 1 as violations, not an error

Linters and formatters use exit code 1 to mean "violations found", not a
crash. commandSemantics fell back to DEFAULT_SEMANTIC for ruff/eslint, so a
run that merely reported lint findings was flagged isError: true and the
model retried the same command up to 3 times before giving up (observed on
Windows with `uvx ruff check --fix`).

Add a LINT_SEMANTIC (exit 1 = violations found, 2+ = real error, mirroring
the existing grep/diff pattern) for ruff and eslint, in both the Bash and
PowerShell tables. Wrapper runners (uvx, npx) inherit the wrapped tool's
semantics only when it resolves to a recognized command — an unrecognized
wrapped tool still falls back to the default, so `uvx <arbitrary>` is not
blanket-treated as non-error.

Adds coverage for ruff/eslint exit codes and the uvx/npx unwrap (including
the unknown-wrapper fallback) in both Bash and PowerShell suites.

* fix(command-semantics): normalize path-prefixed and quoted Bash linters

extractBaseCommand returned the raw first token, so path-prefixed or quoted
invocations (./node_modules/.bin/eslint, "ruff", /usr/bin/uvx ruff, npx
./node_modules/.bin/eslint) fell through to default exit-code semantics and a
linter's exit 1 was mis-reported as an error. Normalize the base and wrapped
command names (strip surrounding quotes and any path prefix) like the
PowerShell implementation does, and match the wrapper by its normalized name.
Adds regression coverage for path-prefixed and quoted linter/wrapper commands.

* fix(command-semantics): normalize Windows .cmd/.bat/.ps1 shims on PowerShell path

extractBaseCommand only stripped .exe, so npm-installed tools and wrappers
invoked via their Windows .cmd shims (eslint.cmd, npx.cmd,
.\node_modules\.bin\eslint.cmd) fell back to DEFAULT_SEMANTIC and reported
exit 1 as an error, regressing the lint-exit-code fix on the PowerShell path.
Broaden the suffix strip to the common PATHEXT executable/shim extensions
(.exe/.cmd/.bat/.ps1) so direct and wrapped .cmd invocations resolve to the
tool/wrapper name. Adds regression coverage for direct, path-prefixed, and
wrapped .cmd forms.

src/tools/BashTool/commandSemantics.test.ts

@@ -160,4 +160,103 @@ describe('interpretCommandResult', () => {
       expect(result.isError).toBe(true)
     })
   })
+
+  // --- ruff / eslint (linters) + uvx / npx wrappers ---
+  describe('linters and wrappers', () => {
+    test('ruff exit code 0 = clean', () => {
+      const result = interpretCommandResult('ruff check .', 0, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('ruff exit code 1 = violations found (not error)', () => {
+      const result = interpretCommandResult('ruff check --fix', 1, 'F401 imported but unused\n', '')
+      expect(result.isError).toBe(false)
+      expect(result.message).toContain('violations')
+    })
+
+    test('ruff exit code 2 = real error', () => {
+      const result = interpretCommandResult('ruff check .', 2, '', 'invalid pyproject config')
+      expect(result.isError).toBe(true)
+    })
+
+    test('eslint exit code 1 = lint problems (not error)', () => {
+      const result = interpretCommandResult('eslint src/', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('eslint exit code 2 = fatal config error', () => {
+      const result = interpretCommandResult('eslint src/', 2, '', 'Cannot read config file')
+      expect(result.isError).toBe(true)
+    })
+
+    test('uvx ruff inherits ruff semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('uvx ruff check --fix', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx eslint inherits eslint semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('npx eslint .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx with flags before the tool still unwraps: exit 1 not error', () => {
+      const result = interpretCommandResult('npx -y eslint .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('uvx wrapping an unrecognized tool falls back to default: exit 1 = error', () => {
+      const result = interpretCommandResult('uvx somecli run', 1, '', '')
+      expect(result.isError).toBe(true)
+    })
+
+    test('bare npx with no recognized tool uses default semantics', () => {
+      const result = interpretCommandResult('npx', 1, '', '')
+      expect(result.isError).toBe(true)
+    })
+
+    test('path-prefixed eslint inherits lint semantics: exit 1 not error', () => {
+      const result = interpretCommandResult(
+        './node_modules/.bin/eslint .',
+        1,
+        '',
+        '',
+      )
+      expect(result.isError).toBe(false)
+    })
+
+    test('quoted linter inherits lint semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('"ruff" check .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('path-prefixed uvx wrapper unwraps to ruff: exit 1 not error', () => {
+      const result = interpretCommandResult(
+        '/usr/bin/uvx ruff check --fix',
+        1,
+        '',
+        '',
+      )
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx wrapping a path-prefixed eslint unwraps: exit 1 not error', () => {
+      const result = interpretCommandResult(
+        'npx ./node_modules/.bin/eslint .',
+        1,
+        '',
+        '',
+      )
+      expect(result.isError).toBe(false)
+    })
+
+    test('path-prefixed linter still surfaces a real error: exit 2 = error', () => {
+      const result = interpretCommandResult(
+        './node_modules/.bin/eslint .',
+        2,
+        '',
+        'Invalid config',
+      )
+      expect(result.isError).toBe(true)
+    })
+  })
 })

src/tools/BashTool/commandSemantics.ts

@@ -25,6 +25,24 @@ const DEFAULT_SEMANTIC: CommandSemantic = (exitCode, _stdout, _stderr) => ({
     exitCode !== 0 ? `Command failed with exit code ${exitCode}` : undefined,
 })
 
+/**
+ * Linters / formatters: 0 = clean, 1 = violations/diffs found (reported in the
+ * output, not a crash), 2+ = a real error (invalid config, bad arguments).
+ * Treating exit 1 as an error makes the model retry a command that already did
+ * its job, so surface it as a non-error result instead.
+ */
+const LINT_SEMANTIC: CommandSemantic = (exitCode, _stdout, _stderr) => ({
+  isError: exitCode >= 2,
+  message: exitCode === 1 ? 'Lint violations found' : undefined,
+})
+
+/**
+ * Wrapper runners that execute another tool (e.g. `uvx ruff check`,
+ * `npx eslint .`). The wrapped tool determines the exit code, so we inherit
+ * its semantics when it is one we recognize.
+ */
+const WRAPPER_COMMANDS = new Set(['uvx', 'npx'])
+
 /**
  * Command-specific semantics
  */
@@ -84,6 +102,12 @@ const COMMAND_SEMANTICS: Map<string, CommandSemantic> = new Map([
     }),
   ],
 
+  // ruff / eslint: 0=clean, 1=lint violations found (reported, not a crash),
+  // 2+=real error (invalid config/args). Also applied to `uvx ruff` / `npx
+  // eslint` via the wrapper unwrap in getCommandSemantic.
+  ['ruff', LINT_SEMANTIC],
+  ['eslint', LINT_SEMANTIC],
+
   // wc, head, tail, cat, etc.: these typically only fail on real errors
   // so we use default semantics
 ])
@@ -95,14 +119,70 @@ function getCommandSemantic(command: string): CommandSemantic {
   // Extract the base command (first word, handling pipes)
   const baseCommand = heuristicallyExtractBaseCommand(command)
   const semantic = COMMAND_SEMANTICS.get(baseCommand)
-  return semantic !== undefined ? semantic : DEFAULT_SEMANTIC
+  if (semantic !== undefined) {
+    return semantic
+  }
+  // uvx/npx run another tool; inherit that tool's semantics when we can
+  // confidently identify a known one (e.g. `uvx ruff check`, `npx -y eslint .`).
+  if (WRAPPER_COMMANDS.has(baseCommand)) {
+    const wrapped = extractWrappedCommand(command, baseCommand)
+    const wrappedSemantic =
+      wrapped !== undefined ? COMMAND_SEMANTICS.get(wrapped) : undefined
+    if (wrappedSemantic !== undefined) {
+      return wrappedSemantic
+    }
+  }
+  return DEFAULT_SEMANTIC
 }
 
 /**
- * Extract just the command name (first word) from a single command string.
+ * For a wrapper invocation (`uvx <tool> ...`, `npx <tool> ...`) return the name
+ * of the wrapped tool — the first non-flag token after the wrapper — so its
+ * exit-code semantics can be applied. Returns undefined when no such token
+ * exists; an unrecognized wrapped name (e.g. a `--from` package) simply falls
+ * back to the default semantic.
+ */
+function extractWrappedCommand(
+  command: string,
+  wrapper: string,
+): string | undefined {
+  const segments = splitCommand_DEPRECATED(command)
+  const lastCommand = segments[segments.length - 1] || command
+  const tokens = lastCommand.trim().split(/\s+/)
+  // Match the wrapper by its normalized name so a resolved or quoted path
+  // (`/usr/bin/uvx`, `"npx"`) still counts as the wrapper.
+  const wrapperIndex = tokens.findIndex(
+    token => extractBaseCommand(token) === wrapper,
+  )
+  if (wrapperIndex === -1) {
+    return undefined
+  }
+  for (let i = wrapperIndex + 1; i < tokens.length; i++) {
+    const token = tokens[i]
+    if (token && !token.startsWith('-')) {
+      // Normalize the wrapped tool too: `npx ./node_modules/.bin/eslint` must
+      // resolve to `eslint` so its lint semantics apply.
+      return extractBaseCommand(token)
+    }
+  }
+  return undefined
+}
+
+/**
+ * Extract just the command name from a single command string, normalized so a
+ * path-prefixed or quoted invocation still maps to a known command. Mirrors the
+ * PowerShell implementation (minus the Windows-only `.exe`/case handling):
+ * `./node_modules/.bin/eslint` → `eslint`, `"ruff"` → `ruff`,
+ * `/usr/bin/uvx` → `uvx`. Otherwise these fall through to the default
+ * exit-code semantics and a linter's exit 1 is mis-reported as an error.
  */
 function extractBaseCommand(command: string): string {
-  return command.trim().split(/\s+/)[0] || ''
+  const firstToken = command.trim().split(/\s+/)[0] || ''
+  // Strip surrounding quotes: "ruff" / 'eslint' → ruff / eslint.
+  const unquoted = firstToken.replace(/^["']|["']$/g, '')
+  // Strip any path prefix (POSIX separator): ./node_modules/.bin/eslint →
+  // eslint, /usr/bin/uvx → uvx.
+  return unquoted.split('/').pop() || unquoted
 }
 
 /**

src/tools/PowerShellTool/commandSemantics.test.ts

@@ -0,0 +1,115 @@
+import { describe, expect, test } from 'bun:test'
+import { interpretCommandResult } from './commandSemantics.js'
+
+// ---------------------------------------------------------------------------
+// interpretCommandResult — PowerShell exit-code semantics per command
+// ---------------------------------------------------------------------------
+
+describe('interpretCommandResult (PowerShell)', () => {
+  describe('default semantics', () => {
+    test('exit code 0 = success', () => {
+      const result = interpretCommandResult('python script.py', 0, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('exit code 1 = error for a plain command', () => {
+      const result = interpretCommandResult('python script.py', 1, '', '')
+      expect(result.isError).toBe(true)
+    })
+  })
+
+  describe('grep / robocopy (existing behavior)', () => {
+    test('grep exit code 1 = no matches (not error)', () => {
+      const result = interpretCommandResult('grep foo file.txt', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('robocopy exit code 1 = files copied (not error)', () => {
+      const result = interpretCommandResult('robocopy src dst', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+  })
+
+  // The reported bug (#1436) was observed on Windows with `uvx ruff check --fix`.
+  describe('linters (ruff / eslint) and uvx / npx wrappers', () => {
+    test('ruff exit code 1 = violations found (not error)', () => {
+      const result = interpretCommandResult('ruff check --fix', 1, 'F401\n', '')
+      expect(result.isError).toBe(false)
+      expect(result.message).toContain('violations')
+    })
+
+    test('ruff exit code 2 = real error', () => {
+      const result = interpretCommandResult('ruff check .', 2, '', 'invalid config')
+      expect(result.isError).toBe(true)
+    })
+
+    test('ruff.exe strips suffix and inherits lint semantics', () => {
+      const result = interpretCommandResult('ruff.exe check .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('eslint exit code 1 = lint problems (not error)', () => {
+      const result = interpretCommandResult('eslint src/', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('eslint exit code 2 = fatal config error', () => {
+      const result = interpretCommandResult('eslint src/', 2, '', 'Cannot read config')
+      expect(result.isError).toBe(true)
+    })
+
+    test('uvx ruff check inherits ruff semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('uvx ruff check --fix', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx eslint inherits eslint semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('npx eslint .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx with flags before the tool still unwraps', () => {
+      const result = interpretCommandResult('npx -y eslint .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('uvx wrapping an unrecognized tool falls back to default: exit 1 = error', () => {
+      const result = interpretCommandResult('uvx somecli run', 1, '', '')
+      expect(result.isError).toBe(true)
+    })
+
+    test('bare npx with no recognized tool uses default semantics', () => {
+      const result = interpretCommandResult('npx', 1, '', '')
+      expect(result.isError).toBe(true)
+    })
+
+    // #1846 review: Windows npm-installed tools/wrappers are invoked via `.cmd`
+    // shims. These must normalize the same way `.exe` does, or the exit-1 lint
+    // fix regresses on the PowerShell path (they fell back to default and
+    // reported isError: true).
+    test('eslint.cmd shim strips suffix and inherits lint semantics', () => {
+      const result = interpretCommandResult('eslint.cmd src/', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('ruff.cmd shim strips suffix and inherits lint semantics', () => {
+      const result = interpretCommandResult('ruff.cmd check .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+
+    test('path-prefixed eslint.cmd shim inherits lint semantics', () => {
+      const result = interpretCommandResult(
+        '.\\node_modules\\.bin\\eslint.cmd .',
+        1,
+        '',
+        '',
+      )
+      expect(result.isError).toBe(false)
+    })
+
+    test('npx.cmd wrapper shim unwraps to eslint semantics: exit 1 not error', () => {
+      const result = interpretCommandResult('npx.cmd eslint .', 1, '', '')
+      expect(result.isError).toBe(false)
+    })
+  })
+})

src/tools/PowerShellTool/commandSemantics.ts

@@ -43,6 +43,24 @@ const GREP_SEMANTIC: CommandSemantic = (exitCode, _stdout, _stderr) => ({
   message: exitCode === 1 ? 'No matches found' : undefined,
 })
 
+/**
+ * Linters / formatters (ruff, eslint): 0 = clean, 1 = violations/diffs found
+ * (reported in the output, not a crash), 2+ = a real error (invalid config,
+ * bad arguments). Treating exit 1 as an error makes the model retry a command
+ * that already did its job.
+ */
+const LINT_SEMANTIC: CommandSemantic = (exitCode, _stdout, _stderr) => ({
+  isError: exitCode >= 2,
+  message: exitCode === 1 ? 'Lint violations found' : undefined,
+})
+
+/**
+ * Wrapper runners that execute another tool (`uvx ruff check`, `npx eslint .`).
+ * The wrapped tool determines $LASTEXITCODE, so we inherit its semantics when
+ * it is one we recognize.
+ */
+const WRAPPER_COMMANDS = new Set(['uvx', 'npx'])
+
 /**
  * Command-specific semantics for external executables.
  * Keys are lowercase command names WITHOUT .exe suffix.
@@ -91,11 +109,18 @@ const COMMAND_SEMANTICS: Map<string, CommandSemantic> = new Map([
             : undefined,
     }),
   ],
+
+  // ruff / eslint (external executables): 1 = lint violations found (reported,
+  // not a crash), 2+ = real error. Also applied to `uvx ruff` / `npx eslint`
+  // via the wrapper unwrap in interpretCommandResult.
+  ['ruff', LINT_SEMANTIC],
+  ['eslint', LINT_SEMANTIC],
 ])
 
 /**
  * Extract the command name from a single pipeline segment.
- * Strips leading `&` / `.` call operators and `.exe` suffix, lowercases.
+ * Strips leading `&` / `.` call operators and Windows executable/shim suffixes
+ * (`.exe`, `.cmd`, `.bat`, `.ps1`), lowercases.
  */
 function extractBaseCommand(segment: string): string {
   // Strip PowerShell call operators: & "cmd", . "cmd"
@@ -106,8 +131,10 @@ function extractBaseCommand(segment: string): string {
   const unquoted = firstToken.replace(/^["']|["']$/g, '')
   // Strip path: C:\bin\grep.exe → grep.exe, .\rg.exe → rg.exe
   const basename = unquoted.split(/[\\/]/).pop() || unquoted
-  // Strip .exe suffix (Windows is case-insensitive)
-  return basename.toLowerCase().replace(/\.exe$/, '')
+  // Strip common Windows executable/shim suffixes so npm `.cmd` shims and other
+  // PATHEXT variants resolve to the tool name (eslint.cmd -> eslint,
+  // npx.cmd -> npx). Windows is case-insensitive.
+  return basename.toLowerCase().replace(/\.(exe|cmd|bat|ps1)$/, '')
 }
 
 /**
@@ -124,6 +151,38 @@ function heuristicallyExtractBaseCommand(command: string): string {
   return extractBaseCommand(last)
 }
 
+/**
+ * Interpret command result based on semantic rules
+ */
+/**
+ * For a wrapper invocation (`uvx <tool> ...`, `npx <tool> ...`) return the
+ * normalized name of the wrapped tool — the first non-flag token after the
+ * wrapper — so its exit-code semantics can be applied. Returns undefined when
+ * no such token exists; an unrecognized wrapped name falls back to default.
+ */
+function extractWrappedCommand(
+  command: string,
+  wrapper: string,
+): string | undefined {
+  const segments = command.split(/[;|]/).filter(s => s.trim())
+  const last = segments[segments.length - 1] || command
+  const tokens = last
+    .trim()
+    .split(/\s+/)
+    .filter(t => t && !/^[&.]$/.test(t))
+  const wrapperIndex = tokens.findIndex(t => extractBaseCommand(t) === wrapper)
+  if (wrapperIndex === -1) {
+    return undefined
+  }
+  for (let i = wrapperIndex + 1; i < tokens.length; i++) {
+    const token = tokens[i]
+    if (token && !token.startsWith('-')) {
+      return extractBaseCommand(token)
+    }
+  }
+  return undefined
+}
+
 /**
  * Interpret command result based on semantic rules
  */
@@ -137,6 +196,12 @@ export function interpretCommandResult(
   message?: string
 } {
   const baseCommand = heuristicallyExtractBaseCommand(command)
-  const semantic = COMMAND_SEMANTICS.get(baseCommand) ?? DEFAULT_SEMANTIC
-  return semantic(exitCode, stdout, stderr)
+  let semantic = COMMAND_SEMANTICS.get(baseCommand)
+  if (semantic === undefined && WRAPPER_COMMANDS.has(baseCommand)) {
+    const wrapped = extractWrappedCommand(command, baseCommand)
+    if (wrapped !== undefined) {
+      semantic = COMMAND_SEMANTICS.get(wrapped)
+    }
+  }
+  return (semantic ?? DEFAULT_SEMANTIC)(exitCode, stdout, stderr)
 }