feat(setup): the guide's consent lands on the change, not the look [spec 03-03]

specs/spec03/03-03-guide-harness.md

@@ -53,6 +53,7 @@
 ## 3. Design
 - **Isolation preserved** (03-01 §2.1): `setting_sources=[]`, `strict_mcp_config=True`, no user skills/MCP. **But built-in tools are ENABLED** and allowlisted (`_GUIDE_BUILTINS` = Bash/Read/Write/Edit/Glob/Grep) — the bounded surface a repair task needs (contrast: find-music runs with `tools=[]`). This is the per-task tool-surface principle: each capability gets exactly what it needs.
 - **Flow**: startup / first music use → deterministic preflight → if broken, murmur tells the user plainly and offers the guide → on opt-in, `SetupGuide.fix_music` runs → Claude Code investigates (Bash), **asks before each change** (SDK `default` mode, routed to the CLI Host), applies the smallest safe fix, verifies → returns an explanation.
+  **Amended (2026-08-12, user decision — consent lands on the change, not the look)**: the offer's `y` already covers *investigation*, so `cliPermission` auto-allows pure reads instead of re-asking — the read-only builtins (Read/Glob/Grep) and Bash commands every segment of which matches a conservative read-only allowlist (`isReadOnlyCommand`, `src/guide.ts`: which/ls/echo/version-reads/brew info-list/…; redirects, backticks, parameter expansion (`$VAR` — only `$?` and an allowlisted `$(...)` pass), or one unknown head disqualify the whole command, falling back to the ask; `brew outdated` is excluded because default Homebrew auto-updates before answering it). Secret-bearing targets (`voice.json`, `.env*`) are never auto-allowed for any tool — an unconsented read would put a credential into the SDK transcript §7.2 keeps it out of. Anything that can mutate — installs, upgrades, Write/Edit, any unrecognized command — still gets the per-action y/N. `bypassPermissions` remains forbidden (the red line stands); auto-allows are recorded in the dev log.
 - **Off the live broadcast loop** (master §3.2 boundary ②): setup/repair is a foreground interaction (first-run, radio not yet broadcasting) or a background job — its exact relationship to the broadcast loop is an open question.
 - **Model**: Opus (repair is judgment-heavy and occasional; the token cost amortizes).
 

src/guide.ts

@@ -99,11 +99,77 @@ export function lineReader(host: Host, quit?: QuitLatch): ReadLine {
   }
 }
 
+// Builtins that can only look, never touch. Write/Edit stay out on purpose.
+const READONLY_TOOLS = new Set(['Read', 'Glob', 'Grep'])
+
+// Where credentials live (spec 03-03 §7.2: voice.json holds the api key,
+// .env* the remote-voice creds). A "read" of these puts a secret into the SDK
+// transcript, so it is never auto-allowed, whatever the tool (codex review).
+const SECRET_BEARING = /\.env|voice\.json/i
+
+// The read-only shapes the guide's diagnosis actually uses. Whole-segment
+// anchors rather than command heads where the head alone is not enough:
+// `yt-dlp --version` reads, `yt-dlp <url>` downloads.
+const READONLY_SEGMENT = [
+  /^which(\s|$)/,
+  /^type\s/,
+  /^command -v\s/,
+  /^ls(\s|$)/,
+  /^echo(\s|$)/,
+  /^printf\s/,
+  /^pwd$/,
+  /^uname(\s|$)/,
+  /^head(\s|$)/,
+  /^tail(\s|$)/,
+  /^wc(\s|$)/,
+  /^yt-dlp --version$/,
+  /^ffmpeg -version$/,
+  /^bun --version$/,
+  /^node --version$/,
+  // `brew outdated` is NOT here: default Homebrew auto-updates its own
+  // metadata before answering it, which mutates state (codex review).
+  /^brew (info|list|config|doctor|--version|--prefix)(\s|$)/,
+  /^uv (--version$|tool list)/,
+  /^pipx (list|--version)/,
+]
+
+// A conservative classifier: is this Bash command incapable of changing the
+// machine? Redirects and backticks disqualify outright; the rest is split at
+// every separator (;, &&, ||, |, &, newline) AND around $(...) so each
+// executable segment is judged on its own — one unknown head poisons the
+// whole command. False negatives just fall back to asking; a false positive
+// would execute silently, so every rule leans strict.
+export function isReadOnlyCommand(command: string): boolean {
+  if (/[><`]/.test(command)) return false
+  // Parameter/env expansion can surface a credential into the transcript
+  // (`echo $MURMUR_TTS_API_KEY`), so `$` is allowed only as `$?` or a `$(...)`
+  // whose inner command is judged like any other segment (codex review).
+  if (/\$(?![?(])/.test(command)) return false
+  if (SECRET_BEARING.test(command)) return false
+  const segments = command
+    .split(/\|\||&&|[;&|\n]|\$\(|\)/)
+    .map((piece) => piece.trim().replace(/^["']+|["']+$/g, '').trim())
+    .filter((piece) => piece !== '')
+  if (segments.length === 0) return false
+  return segments.every((segment) => READONLY_SEGMENT.some((rule) => rule.test(segment)))
+}
+
 // Ask the user via the CLI Host before each tool the guide wants to run, and
 // return the SDK's allow/deny result. Anything but an explicit yes denies.
+// One carve-out (spec 03-03 §7.1): the card's 'y' already covered LOOKING, so
+// pure reads flow without another ask and the per-action consent lands on the
+// CHANGES — a wall of y/N for `which` and `--version` buries the one confirm
+// that matters. The dev log keeps a record of what was auto-allowed.
 export function cliPermission(host: Host, read: ReadLine): CanUseTool {
   return async (toolName, input) => {
     const detail = typeof input.command === 'string' ? input.command : JSON.stringify(input)
+    if (
+      (READONLY_TOOLS.has(toolName) && !SECRET_BEARING.test(JSON.stringify(input))) ||
+      (toolName === 'Bash' && typeof input.command === 'string' && isReadOnlyCommand(input.command))
+    ) {
+      host.debug?.(`guide auto-allowed read-only [${toolName}]: ${detail}`)
+      return { behavior: 'allow' }
+    }
     // One self-contained ask: a docked "allow?" with the command left behind
     // in the log would ask the user to approve something they cannot see.
     ask(host, `setup assistant wants to run [${toolName}]: ${detail}\nallow? [y/N]`, 'consent')

test/guide.test.ts

@@ -1,6 +1,12 @@
 import { describe, expect, it } from 'vitest'
 
-import { cliConversation, cliPermission, lineReader, quitLatch } from '../src/guide.ts'
+import {
+  cliConversation,
+  cliPermission,
+  isReadOnlyCommand,
+  lineReader,
+  quitLatch,
+} from '../src/guide.ts'
 import type { AskKind, Host } from '../src/host.ts'
 
 // A host with scripted keyboard lines (the same stdin the Director uses).
@@ -104,6 +110,106 @@ describe('cliPermission (spec 03-03 §2 — route the ask, never own the semanti
   })
 })
 
+// The card's 'y' already covered LOOKING (the user asked to be walked through
+// a fix); what still needs per-action consent is CHANGE. Pure investigation
+// flows without another ask — a wall of y/N for `which` and `--version` reads
+// as noise and buries the one confirm that matters.
+describe('cliPermission — read-only investigation flows without asking (spec 03-03 §7.1)', () => {
+  it('auto-allows the diagnostics the guide actually runs, without touching the keyboard', async () => {
+    // No scripted lines: if any of these asked, the read would hang the test.
+    const { host, asks } = fakeHost([], { docked: true })
+    const ask = cliPermission(host, lineReader(host))
+    for (const command of [
+      'which -a yt-dlp; echo "---"; yt-dlp --version; echo "---"; ls -l "$(which yt-dlp)"',
+      'echo "exit: $?"; echo "---"; brew info yt-dlp | head -5',
+      'uv tool list && pipx list',
+      'ffmpeg -version',
+      'brew --prefix && brew list --versions yt-dlp',
+    ]) {
+      expect(await ask('Bash', { command }, askOptions)).toEqual({ behavior: 'allow' })
+    }
+    expect(asks).toEqual([])
+  })
+
+  it('secret-bearing reads stay behind consent, whatever the tool (codex review)', async () => {
+    // The out-of-band secret flow (§7.2) exists so credentials never enter the
+    // SDK transcript; an auto-allowed read of the config or the env would put
+    // them there without anyone agreeing to it.
+    const { host, asks } = fakeHost(['', '', '', ''], { docked: true })
+    const ask = cliPermission(host, lineReader(host))
+    for (const [tool, input] of [
+      ['Bash', { command: 'echo $MURMUR_TTS_API_KEY' }],
+      ['Read', { file_path: '/Users/zach/.murmur/voice.json' }],
+      ['Read', { file_path: '/Users/zach/.personal/murmur/.env' }],
+      ['Grep', { pattern: 'apiKey', path: '/Users/zach/.murmur/voice.json' }],
+    ] as const) {
+      expect(await ask(tool, input, askOptions)).toMatchObject({ behavior: 'deny' })
+    }
+    expect(asks).toHaveLength(4)
+  })
+
+  it('read-only builtins pass without a question; Write and Edit still ask', async () => {
+    const { host, asks } = fakeHost(['n'], { docked: true })
+    const ask = cliPermission(host, lineReader(host))
+    expect(await ask('Read', { file_path: '/tmp/x' }, askOptions)).toEqual({ behavior: 'allow' })
+    expect(await ask('Glob', { pattern: '**/*.ts' }, askOptions)).toEqual({ behavior: 'allow' })
+    expect(await ask('Grep', { pattern: 'x' }, askOptions)).toEqual({ behavior: 'allow' })
+    expect(asks).toEqual([])
+    expect(await ask('Write', { file_path: '/tmp/x' }, askOptions)).toMatchObject({
+      behavior: 'deny',
+    })
+    expect(asks).toHaveLength(1)
+  })
+
+  it('anything that can mutate still asks: the consent stays on the change', async () => {
+    const { host, infos } = fakeHost(['y', ''])
+    const ask = cliPermission(host, lineReader(host))
+    expect(await ask('Bash', { command: 'brew upgrade yt-dlp' }, askOptions)).toEqual({
+      behavior: 'allow',
+    })
+    expect(await ask('Bash', { command: 'brew install ffmpeg' }, askOptions)).toMatchObject({
+      behavior: 'deny',
+    })
+    expect(infos.join('\n')).toContain('brew upgrade yt-dlp')
+  })
+})
+
+describe('isReadOnlyCommand — the conservative classifier', () => {
+  it('refuses unknown heads, redirects, chained mutations, and mutating substitutions', () => {
+    for (const command of [
+      'brew upgrade yt-dlp',
+      'rm -rf /tmp/x',
+      'echo hi > /tmp/x', // a redirect writes
+      'ls $(curl example.com)', // unknown head inside $()
+      'which yt-dlp && brew upgrade yt-dlp', // one bad segment poisons the chain
+      'ls & rm -rf /tmp/x', // a single & backgrounds; the second head must be seen
+      'yt-dlp https://example.com', // beyond --version, yt-dlp downloads
+      'echo `rm -rf /tmp/x`', // backticks are substitution too
+      '', // nothing is not a read
+      'brew outdated yt-dlp', // brew auto-updates its own metadata first (codex review)
+      'echo $MURMUR_TTS_API_KEY', // env expansion can surface a credential (codex review)
+      'ls "$HOME/.murmur"', // same: parameter expansion is not provably a read
+      'cat voice.json', // the secret-bearing config never auto-reads
+    ]) {
+      expect(isReadOnlyCommand(command), command).toBe(false)
+    }
+  })
+
+  it('accepts version reads and package queries', () => {
+    for (const command of [
+      'yt-dlp --version',
+      'ffmpeg -version',
+      'brew list',
+      'brew --prefix',
+      'pwd',
+      'uname -a',
+      'command -v yt-dlp',
+    ]) {
+      expect(isReadOnlyCommand(command), command).toBe(true)
+    }
+  })
+})
+
 describe('cliConversation', () => {
   it('returns the typed reply; empty or /done or q ends it', async () => {
     const { host } = fakeHost(['  the quick fix please  ', '', '/done', 'Q'])