Merge pull request #954 from getagentseal/fix/941-bash-read-verify

fix(insights): count read-shaped shell commands as reads, not as verification

src/bash-utils.ts

@@ -62,3 +62,54 @@ export function extractBashCommands(rawCommand: string): string[] {
 
   return commands
 }
+
+// Read-shaped shell commands: they inspect state and cannot modify the
+// worktree, so (1) the read-edit-ratio detector counts them as reads and
+// (2) retry detection does NOT treat them as a verification step between two
+// edits of the same file (#941 — both detectors previously disagreed about
+// the same Bash call, scoring rg-first workflows as reckless AND reworked).
+const READ_ONLY_BASH = new Set([
+  'rg', 'grep', 'egrep', 'fgrep', 'ag',
+  'cat', 'head', 'tail', 'less', 'more',
+  'ls', 'find', 'fd', 'tree',
+  'wc', 'stat', 'file', 'du', 'df',
+  'which', 'type', 'pwd', 'printenv', 'env',
+  'readlink', 'realpath', 'basename', 'dirname',
+  'jq', 'diff',
+])
+
+// git subcommands that only inspect history/state. Deliberately excludes
+// anything that can create or mutate under any flag (branch, tag, stash,
+// remote), so a mutation is never misread as a read.
+const GIT_READ_SUBCOMMANDS = new Set([
+  'log', 'diff', 'status', 'show', 'blame', 'grep',
+  'shortlog', 'describe', 'rev-parse', 'ls-files',
+])
+
+/// True when EVERY segment of the raw command line (split on &&, ;, |) is a
+/// read-only inspection command. A single mutating segment makes the whole
+/// call non-read (`cat x && sed -i ...` edits). Unknown commands are
+/// non-read: the conservative default both call sites want.
+export function isReadShapedBashCommand(rawCommand: string): boolean {
+  if (!rawCommand || !rawCommand.trim()) return false
+  const stripped = stripQuotedStrings(stripAnsi(rawCommand))
+  const segments = stripped.split(/\s*(?:&&|;|\|)\s*/)
+  let sawCommand = false
+  for (const segment of segments) {
+    const trimmed = segment.trim()
+    if (!trimmed) continue
+    const tokens = trimmed.split(/\s+/)
+    let i = 0
+    while (i < tokens.length && (/^\w+=/.test(tokens[i]!) || COMMAND_PREFIXES.has(basename(tokens[i]!)))) i++
+    const base = i < tokens.length ? basename(tokens[i]!) : ''
+    if (!base) continue
+    sawCommand = true
+    if (base === 'git') {
+      const sub = tokens[i + 1]
+      if (!sub || !GIT_READ_SUBCOMMANDS.has(sub)) return false
+      continue
+    }
+    if (!READ_ONLY_BASH.has(base)) return false
+  }
+  return sawCommand
+}

src/classifier.ts

@@ -1,4 +1,5 @@
 import type { ClassifiedTurn, ParsedTurn, TaskCategory, ToolCall } from './types.js'
+import { isReadShapedBashCommand } from './bash-utils.js'
 
 const TEST_PATTERNS = /\b(test|pytest|vitest|jest|mocha|spec|coverage|npm\s+test|npx\s+vitest|npx\s+jest)\b/i
 const GIT_PATTERNS = /\bgit\s+(push|pull|commit|merge|rebase|checkout|branch|stash|log|diff|status|add|reset|cherry-pick|tag)\b/i
@@ -170,7 +171,13 @@ function countRetries(turn: ParsedTurn): number {
   steps.forEach((step, i) => {
     for (const call of step) {
       if (BASH_TOOLS.has(call.tool)) {
-        lastVerifyStep = i
+        // A read-shaped shell command (rg/cat/git log) is a lookup, not a
+        // verification of the previous edit: edit -> grep -> edit is research,
+        // and counting it as a retry penalized bash-first workflows (#941).
+        // A command we cannot classify keeps the old behavior.
+        if (!call.command || !isReadShapedBashCommand(call.command)) {
+          lastVerifyStep = i
+        }
       }
       if (EDIT_TOOLS.has(call.tool)) {
         const fileKey = call.file ?? '__no_file__'

src/optimize.ts

@@ -1,4 +1,5 @@
 import chalk from 'chalk'
+import { isReadShapedBashCommand } from './bash-utils.js'
 import { readdir, stat } from 'fs/promises'
 import { existsSync, statSync } from 'fs'
 import { basename, join } from 'path'
@@ -383,6 +384,10 @@ function compactOptimizeInput(name: string, input: unknown): Record<string, unkn
     const filePath = cappedString(raw['file_path'], OPTIMIZE_TEXT_CAP)
     return filePath ? { file_path: filePath } : {}
   }
+  if (BASH_TOOL_NAMES.has(name)) {
+    const command = cappedString(raw['command'], OPTIMIZE_TEXT_CAP)
+    return command ? { command } : {}
+  }
   if (name === 'Agent' || name === 'Task') {
     const subagentType = cappedString(raw['subagent_type'])
     return subagentType ? { subagent_type: subagentType } : {}
@@ -2189,6 +2194,7 @@ export function detectBloatedClaudeMd(projectCwds: Set<string>): WasteFinding |
 
 export const READ_TOOL_NAMES = new Set(['Read', 'Grep', 'Glob', 'FileReadTool', 'GrepTool', 'GlobTool'])
 export const EDIT_TOOL_NAMES = new Set(['Edit', 'Write', 'FileEditTool', 'FileWriteTool', 'NotebookEdit'])
+export const BASH_TOOL_NAMES = new Set(['Bash', 'BashTool', 'PowerShellTool'])
 
 export function detectLowReadEditRatio(calls: ToolCall[]): WasteFinding | null {
   let reads = 0
@@ -2199,6 +2205,12 @@ export function detectLowReadEditRatio(calls: ToolCall[]): WasteFinding | null {
     if (READ_TOOL_NAMES.has(call.name)) {
       reads++
       if (call.recent) recentReads++
+    } else if (BASH_TOOL_NAMES.has(call.name) && typeof call.input['command'] === 'string' && isReadShapedBashCommand(call.input['command'])) {
+      // A session that looks things up with rg/cat/git log IS reading (#941):
+      // ignoring shell reads scored disciplined rg-first workflows as
+      // reckless editors (90%+ of real reads were invisible to this ratio).
+      reads++
+      if (call.recent) recentReads++
     } else if (EDIT_TOOL_NAMES.has(call.name)) {
       edits++
       if (call.recent) recentEdits++

tests/bash-commands.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from 'vitest'
-import { extractBashCommands } from '../src/bash-utils.js'
+import { extractBashCommands, isReadShapedBashCommand } from '../src/bash-utils.js'
 import { BASH_TOOLS } from '../src/classifier.js'
 
 describe('extractBashCommands', () => {
@@ -117,3 +117,33 @@ describe('BASH_TOOLS', () => {
   it('recognizes BashTool', () => { expect(BASH_TOOLS.has('BashTool')).toBe(true) })
   it('rejects unknown tools', () => { expect(BASH_TOOLS.has('Read')).toBe(false) })
 })
+
+describe('isReadShapedBashCommand (#941)', () => {
+  it('accepts single read commands and read-only git subcommands', () => {
+    expect(isReadShapedBashCommand('rg -n "x" src/')).toBe(true)
+    expect(isReadShapedBashCommand('cat file.ts')).toBe(true)
+    expect(isReadShapedBashCommand('git log --oneline -5')).toBe(true)
+    expect(isReadShapedBashCommand('git diff HEAD~1')).toBe(true)
+    expect(isReadShapedBashCommand('VAR=1 sudo head -c 100 f')).toBe(true)
+  })
+
+  it('accepts pipelines where every segment reads', () => {
+    expect(isReadShapedBashCommand('grep -r "x" src | head -20')).toBe(true)
+    expect(isReadShapedBashCommand('git log --oneline && git status')).toBe(true)
+  })
+
+  it('rejects any mutating or unknown segment', () => {
+    expect(isReadShapedBashCommand('npm test')).toBe(false)
+    expect(isReadShapedBashCommand('sed -i s/a/b/ x.ts')).toBe(false)
+    expect(isReadShapedBashCommand('cat x && rm -rf dist')).toBe(false)
+    expect(isReadShapedBashCommand('git commit -m "x"')).toBe(false)
+    expect(isReadShapedBashCommand('git branch new-branch')).toBe(false)
+    expect(isReadShapedBashCommand('git')).toBe(false)
+    expect(isReadShapedBashCommand('')).toBe(false)
+    expect(isReadShapedBashCommand('   ')).toBe(false)
+  })
+
+  it('is not fooled by read-command names inside quoted strings', () => {
+    expect(isReadShapedBashCommand('echo "cat file"')).toBe(false)
+  })
+})

tests/classifier.test.ts

@@ -153,6 +153,28 @@ describe('classifyTurn — feature vs debugging precedence (#196)', () => {
 })
 
 describe('classifyTurn — retry detection via toolSequence', () => {
+  it('does not count a read-shaped shell lookup between edits as a retry (#941)', () => {
+    // edit -> rg -> edit is research, not a failed verification loop.
+    const call = makeCall({ tools: ['Edit', 'Bash'] })
+    call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash', command: 'rg -n "helper" src/' }], [{ tool: 'Edit', file: 'a.ts' }]]
+    const turn = makeTurn([call], 'fix the build')
+    expect(classifyTurn(turn).retries).toBe(0)
+  })
+
+  it('still counts a verification-shaped shell command between edits as a retry (#941)', () => {
+    const call = makeCall({ tools: ['Edit', 'Bash'] })
+    call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash', command: 'npm test' }], [{ tool: 'Edit', file: 'a.ts' }]]
+    const turn = makeTurn([call], 'fix the build')
+    expect(classifyTurn(turn).retries).toBe(1)
+  })
+
+  it('keeps counting command-less bash steps as verification (unknown stays conservative)', () => {
+    const call = makeCall({ tools: ['Edit', 'Bash'] })
+    call.toolSequence = [[{ tool: 'Edit', file: 'a.ts' }], [{ tool: 'Bash' }], [{ tool: 'Edit', file: 'a.ts' }]]
+    const turn = makeTurn([call], 'fix the build')
+    expect(classifyTurn(turn).retries).toBe(1)
+  })
+
   it('detects retries from multi-call turns (Claude-style)', () => {
     const turn = makeTurn([
       makeCall({ tools: ['Edit'] }),

tests/optimize.test.ts

@@ -234,6 +234,30 @@ describe('detectDuplicateReads', () => {
 })
 
 describe('detectLowReadEditRatio', () => {
+  it('counts read-shaped bash commands as reads (#941)', () => {
+    // 10 edits with 40 rg/cat/git-log lookups through the shell: a healthy
+    // 4:1 workflow that previously read as 0 reads and fired at high impact.
+    const calls = [
+      ...Array.from({ length: 20 }, () => call('Bash', { command: 'rg -n "pattern" src/' })),
+      ...Array.from({ length: 10 }, () => call('Bash', { command: 'cat src/parser.ts | head -50' })),
+      ...Array.from({ length: 10 }, () => call('Bash', { command: 'git log --oneline -5' })),
+      ...Array.from({ length: 10 }, () => call('Edit', {})),
+    ]
+    expect(detectLowReadEditRatio(calls)).toBeNull()
+  })
+
+  it('does not count mutating or unclassifiable bash as reads (#941)', () => {
+    const calls = [
+      ...Array.from({ length: 20 }, () => call('Bash', { command: 'npm test' })),
+      ...Array.from({ length: 10 }, () => call('Bash', { command: 'cat notes.md && sed -i s/a/b/ src/x.ts' })),
+      ...Array.from({ length: 10 }, () => call('Bash', {})),
+      ...Array.from({ length: 10 }, () => call('Edit', {})),
+    ]
+    const finding = detectLowReadEditRatio(calls)
+    expect(finding).not.toBeNull()
+    expect(finding!.explanation).toContain('0 reads')
+  })
+
   it('returns null below minimum edit count', () => {
     const calls = [
       call('Edit', {}),