From 47d0866f04d6ba52011f9b9026edfde604cb2e98 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 30 Sep 2026 23:15:39 +0300 Subject: [PATCH 1/4] fix: never run a repository's core.fsmonitor on git index reads git runs the command a repository's config names in core.fsmonitor whenever it reads the index - ls-files, status and diff included. The hooks and the HUD run inside arbitrary repositories, so each unguarded index read executed code the repository chose. Every remaining index read now passes -c core.fsmonitor=false (D-NO-FSMONITOR), matching resolve-settings and verify-evidence: - json-helper listGitTrackedFiles (assign-anchor collision scan) - pre-compact-memory git status / git diff - background-memory-update git status / git diff - the HUD's gitExec wrapper (status and diff on every prompt) Regression tests arm a real repository with a recording fsmonitor hook and assert it never runs while the reported git state stays correct, with a known-bad unguarded probe proving the hook is live. --- .../scripts/hooks/background-memory-update | 6 +- src/assets/scripts/hooks/json-helper.cjs | 7 +- src/assets/scripts/hooks/pre-compact-memory | 9 +- src/assets/scripts/resolve-settings.cjs | 4 +- src/assets/scripts/verify-evidence.cjs | 2 +- src/hud/git.ts | 11 +- tests/decisions/ledger-ops.test.ts | 41 ++++- tests/guards/heredoc-quoting.test.ts | 2 +- tests/integration/hud-git.test.ts | 54 +++++- tests/memory-hooks-fsmonitor.test.ts | 154 ++++++++++++++++++ 10 files changed, 277 insertions(+), 13 deletions(-) create mode 100644 tests/memory-hooks-fsmonitor.test.ts diff --git a/src/assets/scripts/hooks/background-memory-update b/src/assets/scripts/hooks/background-memory-update index e8383d293..72df65cb8 100755 --- a/src/assets/scripts/hooks/background-memory-update +++ b/src/assets/scripts/hooks/background-memory-update @@ -399,9 +399,11 @@ if cd "$CWD" 2>/dev/null && git rev-parse --git-dir >/dev/null 2>&1; then if [ -z "$BRANCH" ] && [ -n "$HEAD_SHA" ]; then BRANCH="(detached)" fi - GIT_STATUS=$(git status --short 2>/dev/null | head -20) + # D-NO-FSMONITOR (pre-compact-memory): the index reads turn off the + # repository's `core.fsmonitor` command, which git would otherwise run. + GIT_STATUS=$(git -c core.fsmonitor=false status --short 2>/dev/null | head -20) GIT_LOG=$(git log --oneline -5 2>/dev/null || echo "") - GIT_DIFF=$(git diff --stat HEAD 2>/dev/null | tail -10) + GIT_DIFF=$(git -c core.fsmonitor=false diff --stat HEAD 2>/dev/null | tail -10) GIT_STATE="Branch: ${BRANCH} HEAD: ${HEAD_SHA} Recent commits: diff --git a/src/assets/scripts/hooks/json-helper.cjs b/src/assets/scripts/hooks/json-helper.cjs index e7d076237..ac2bcda0f 100755 --- a/src/assets/scripts/hooks/json-helper.cjs +++ b/src/assets/scripts/hooks/json-helper.cjs @@ -199,11 +199,16 @@ function isCollisionScanExcluded(relPath) { * project root is not a git working tree or the `git` binary is unavailable; * callers fall back to `listFsWalkFiles`. * + * D-NO-FSMONITOR: `ls-files` reads the index, and reading the index runs the + * command a repository's config names in `core.fsmonitor` — code chosen by the + * repository this hook runs inside. The call turns it off for itself + * (`-c core.fsmonitor=false`), so the listing stays a pure read. + * * @param {string} projectRoot * @returns {string[]} project-relative paths */ function listGitTrackedFiles(projectRoot) { - const out = execFileSync('git', ['ls-files', '-z'], { + const out = execFileSync('git', ['-c', 'core.fsmonitor=false', 'ls-files', '-z'], { cwd: projectRoot, stdio: ['ignore', 'pipe', 'ignore'], }); diff --git a/src/assets/scripts/hooks/pre-compact-memory b/src/assets/scripts/hooks/pre-compact-memory index 1c29727ad..b9419a288 100644 --- a/src/assets/scripts/hooks/pre-compact-memory +++ b/src/assets/scripts/hooks/pre-compact-memory @@ -91,9 +91,14 @@ if cd "$CWD" 2>/dev/null && git rev-parse --git-dir >/dev/null 2>&1; then if [ -z "$GIT_BRANCH" ] && is_hex_sha "$GIT_HEAD_SHA" 40 40; then GIT_BRANCH="(detached)" fi - GIT_STATUS=$(git status --porcelain 2>/dev/null | head -30 || echo "") + # D-NO-FSMONITOR: `status` and `diff` read the index, and reading the index + # runs the command the repository's config names in `core.fsmonitor` — code + # chosen by the repository this hook runs inside. Each index read turns it + # off for itself (`-c core.fsmonitor=false`); rev-parse, branch and log never + # read the index. + GIT_STATUS=$(git -c core.fsmonitor=false status --porcelain 2>/dev/null | head -30 || echo "") GIT_LOG=$(git log --oneline -10 2>/dev/null || echo "") - GIT_DIFF_STAT=$(git diff --stat HEAD 2>/dev/null || echo "") + GIT_DIFF_STAT=$(git -c core.fsmonitor=false diff --stat HEAD 2>/dev/null || echo "") dbg "GIT_BRANCH=$GIT_BRANCH HEAD=$GIT_HEAD_SHA" fi diff --git a/src/assets/scripts/resolve-settings.cjs b/src/assets/scripts/resolve-settings.cjs index 6a442713e..25f5abb26 100644 --- a/src/assets/scripts/resolve-settings.cjs +++ b/src/assets/scripts/resolve-settings.cjs @@ -671,8 +671,8 @@ function gitToplevel(exec, dir) { * which reads the index without refreshing or writing it. Reading the index * runs a configured `core.fsmonitor` hook — an arbitrary command from the * repository's config, and on macOS the builtin daemon's start-up — so the - * call turns it off for itself (`-c core.fsmonitor=false`): this resolver runs - * from session hooks and must stay a pure read. + * call turns it off for itself (`-c core.fsmonitor=false`, D-NO-FSMONITOR): + * this resolver runs from session hooks and must stay a pure read. * tracked exit 0 * untracked any other answered exit (1: no such index entry; outside a * repository git answers 128, and there is nothing to track) diff --git a/src/assets/scripts/verify-evidence.cjs b/src/assets/scripts/verify-evidence.cjs index 58466ab9d..7db0d075f 100644 --- a/src/assets/scripts/verify-evidence.cjs +++ b/src/assets/scripts/verify-evidence.cjs @@ -517,7 +517,7 @@ function gh(io, args, maxBuffer) { /** * Every git call: `-c core.fsmonitor=false` first, so no repository-configured - * hook runs, and bounded by the deadline. + * hook runs (D-NO-FSMONITOR), and bounded by the deadline. * * @param {Io} io * @param {readonly string[]} args diff --git a/src/hud/git.ts b/src/hud/git.ts index c2a1a8b30..d8bfd6613 100644 --- a/src/hud/git.ts +++ b/src/hud/git.ts @@ -13,8 +13,17 @@ function shellExec(cmd: string, args: string[], cwd: string): Promise { }); } +/** + * Every HUD git call: `-c core.fsmonitor=false` first. + * + * D-NO-FSMONITOR: `status` and `diff` read the index, and reading the index + * runs the command a repository's config names in `core.fsmonitor` — code + * chosen by whatever repository the status line is drawn in, on every prompt. + * The override keeps each call a pure read; calls that never touch the index + * carry it too, so no new call can forget it. + */ function gitExec(args: string[], cwd: string): Promise { - return shellExec('git', args, cwd); + return shellExec('git', ['-c', 'core.fsmonitor=false', ...args], cwd); } /** diff --git a/tests/decisions/ledger-ops.test.ts b/tests/decisions/ledger-ops.test.ts index bdd17409c..3d6704ff1 100644 --- a/tests/decisions/ledger-ops.test.ts +++ b/tests/decisions/ledger-ops.test.ts @@ -13,7 +13,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { createRequire } from 'module'; -import { execSync } from 'child_process'; +import { execFileSync, execSync, spawnSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -1650,6 +1650,45 @@ describe('E4: pre-mint collision guard', () => { expect(after.equals(before)).toBe(true); }); + it('lists tracked files without running the repository\'s core.fsmonitor hook (D-NO-FSMONITOR)', () => { + writeLedger(tmpDir, [makeLedgerRow({ anchor_id: 'ADR-001' })]); + // A gitignored file citing the same id: the fs-walk fallback would report it, + // `git ls-files` never does — so its absence proves the git listing answered. + fs.writeFileSync(path.join(tmpDir, '.gitignore'), 'ignored/\n'); + fs.mkdirSync(path.join(tmpDir, 'ignored'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'ignored', 'scratch.md'), 'ADR-002 scribble.\n'); + initGitRepoWithFile(tmpDir, 'docs/design.md', 'See ADR-002 for the rationale.\n'); + writeLog(tmpDir, [makeObsRow({ id: 'obs_fsmonitor', type: 'decision', status: 'ready' })]); + + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'aa-fsmonitor-hook-')); + const home = path.join(outside, 'home'); + fs.mkdirSync(home); + const marker = path.join(outside, 'fsmonitor-ran'); + const hook = path.join(outside, 'fsmonitor-hook.sh'); + fs.writeFileSync(hook, `#!/bin/sh\necho ran >> '${marker}'\nexit 1\n`); + fs.chmodSync(hook, 0o755); + const env = { ...COLLISION_GIT_ENV, HOME: home }; + execFileSync('git', ['config', 'core.fsmonitor', hook], { cwd: tmpDir, env }); + + try { + const run = spawnSync('node', [JSON_HELPER_BIN, 'assign-anchor', 'decision', 'obs_fsmonitor'], { + cwd: tmpDir, + env, + encoding: 'utf8', + }); + expect(run.status).not.toBe(0); + expect(run.stderr).toContain('docs/design.md:1'); + expect(run.stderr).not.toContain('scratch.md'); + expect(fs.existsSync(marker), 'assign-anchor ran the fsmonitor hook').toBe(false); + + // Known-bad probe: the same index read without the override runs the hook. + execFileSync('git', ['ls-files', '-z'], { cwd: tmpDir, env, stdio: 'ignore' }); + expect(fs.existsSync(marker)).toBe(true); + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + it('refuses to mint when the candidate id is cited in a non-git project (fs-walk fallback)', () => { writeLog(tmpDir, [makeObsRow({ id: 'obs_collide_nogit', type: 'decision', status: 'ready' })]); fs.mkdirSync(path.join(tmpDir, 'docs'), { recursive: true }); diff --git a/tests/guards/heredoc-quoting.test.ts b/tests/guards/heredoc-quoting.test.ts index 296b2e921..3c52d8014 100644 --- a/tests/guards/heredoc-quoting.test.ts +++ b/tests/guards/heredoc-quoting.test.ts @@ -79,7 +79,7 @@ const SCANNED_EXTENSIONS: readonly string[] = ['.md', '.mds', '.sh', '.bash']; */ const KNOWN_UNQUOTED_HEREDOCS: readonly string[] = [ 'src/assets/scripts/hooks/background-memory-update:313', - 'src/assets/scripts/hooks/background-memory-update:422', + 'src/assets/scripts/hooks/background-memory-update:424', 'src/assets/scripts/hooks/capture-question:157', ]; diff --git a/tests/integration/hud-git.test.ts b/tests/integration/hud-git.test.ts index 61a3744de..801915fe0 100644 --- a/tests/integration/hud-git.test.ts +++ b/tests/integration/hud-git.test.ts @@ -23,7 +23,7 @@ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { execFileSync } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; @@ -819,7 +819,7 @@ describe('gatherGitStatus — dirty-tree ahead/filesChanged asymmetry (Shape M)' .filter(args => args.includes('status')); expect(statusCalls).toHaveLength(1); - expect(statusCalls[0]).toEqual(['--no-optional-locks', 'status', '--porcelain']); + expect(statusCalls[0]).toEqual(['-c', 'core.fsmonitor=false', '--no-optional-locks', 'status', '--porcelain']); // The flag must not break the command: a rejected option would yield '' → clean. expect(live?.dirty).toBe(true); expect(live?.staged).toBe(true); @@ -917,3 +917,53 @@ describe('gatherGitStatus — maxBuffer overflow degrades gracefully', () => { } }); }); + +describe('gatherGitStatus — never runs the repository\'s core.fsmonitor hook (D-NO-FSMONITOR)', () => { + /** + * git runs the command a repository's config names in `core.fsmonitor` whenever + * it reads the index — the HUD's `status` and `diff` included — and the HUD runs + * in whatever repository the status line is drawn in, on every prompt. The fixture + * hook records its own run; the HUD must report a dirty tree and its diff without + * running it, and a known-bad unguarded `status` in the same repository must. + */ + it('reports a dirty tree and its diff with the hook never run', async () => { + const base = mkdtempSync(join(tmpdir(), 'hud-fsmonitor-')); + const repo = join(base, 'repo'); + const home = join(base, 'home'); + const marker = join(base, 'fsmonitor-ran'); + const hook = join(base, 'fsmonitor-hook.sh'); + try { + mkdirSync(repo); + mkdirSync(home); + git(repo, ['init', '-q', '-b', 'main']); + writeFileSync(join(repo, 'tracked.txt'), 'one\n'); + git(repo, ['add', 'tracked.txt']); + git(repo, ['commit', '-q', '-m', 'init']); + // A feature branch, so the HUD compares against main and runs its diff too. + git(repo, ['switch', '-q', '-c', 'feature']); + writeFileSync(join(repo, 'tracked.txt'), 'one\ntwo\n'); + // An untracked file makes the tree dirty on a `??` line, which the status + // parse reads independently of the modified tracked file's line. + writeFileSync(join(repo, 'untracked-probe.txt'), 'new\n'); + writeFileSync(hook, `#!/bin/sh\necho ran >> '${marker}'\nexit 1\n`, { mode: 0o755 }); + git(repo, ['config', 'core.fsmonitor', hook]); + + vi.stubEnv('HOME', home); + vi.stubEnv('GIT_CONFIG_NOSYSTEM', '1'); + vi.stubEnv('GIT_CONFIG_GLOBAL', '/dev/null'); + const status = await gatherGitStatus(repo); + + expect(existsSync(marker), 'the HUD ran the fsmonitor hook').toBe(false); + expect(status?.branch).toBe('feature'); + expect(status?.dirty).toBe(true); + expect(status?.filesChanged).toBe(1); + + // Known-bad probe: the same index read without the override runs the hook. + git(repo, ['status', '--porcelain']); + expect(existsSync(marker), 'the fixture hook is live').toBe(true); + } finally { + vi.unstubAllEnvs(); + rmSync(base, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/memory-hooks-fsmonitor.test.ts b/tests/memory-hooks-fsmonitor.test.ts new file mode 100644 index 000000000..47076661f --- /dev/null +++ b/tests/memory-hooks-fsmonitor.test.ts @@ -0,0 +1,154 @@ +/** + * D-NO-FSMONITOR — the memory hooks read a repository's index without running + * its `core.fsmonitor` command. (The HUD's case lives with its real-repo tests + * in tests/integration/hud-git.test.ts; json-helper's in + * tests/decisions/ledger-ops.test.ts.) + * + * git runs the command a repository's config names in `core.fsmonitor` + * whenever it reads the index — `status`, `diff` and `ls-files` included — so + * an index read inside a cloned repository is code execution chosen by that + * repository. Each site that reads the index passes `-c core.fsmonitor=false`. + * + * Every test arms a real repository with a hook that records its own run, runs + * the site, and asserts both halves: the hook never ran, and the git state the + * site reports is still correct. A known-bad probe then runs the same index + * read without the override and sees the hook fire, so a green run cannot come + * from a hook git never calls. Temp dirs only; every spawn gets a temp HOME. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { execFileSync, execSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { runHook } from './shell-hooks-helpers.js'; + +const HOOKS_DIR = path.resolve(__dirname, '..', 'src', 'assets', 'scripts', 'hooks'); +const PRE_COMPACT_HOOK = path.join(HOOKS_DIR, 'pre-compact-memory'); +const BACKGROUND_UPDATER = path.join(HOOKS_DIR, 'background-memory-update'); +/** Each case spawns a full hook plus several git calls; slow under a loaded suite. */ +const HOOK_TIMEOUT_MS = 30_000; + +/** A sandbox: the repository, a HOME, and the fsmonitor hook's run marker. */ +interface Sandbox { + base: string; + repo: string; + home: string; + marker: string; + env: NodeJS.ProcessEnv; +} + +function makeSandbox(prefix: string): Sandbox { + const base = fs.mkdtempSync(path.join(os.tmpdir(), `${prefix}-`)); + const repo = path.join(base, 'repo'); + const home = path.join(base, 'home'); + fs.mkdirSync(repo); + fs.mkdirSync(home); + const env: NodeJS.ProcessEnv = { + ...process.env, + HOME: home, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@test.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@test.com', + }; + return { base, repo, home, marker: path.join(base, 'fsmonitor-ran'), env }; +} + +function git(sb: Sandbox, args: string[]): void { + execFileSync('git', args, { cwd: sb.repo, env: sb.env, stdio: 'ignore' }); +} + +/** + * One commit, then an untracked file and a modified tracked file, then the + * hook — armed last so the fixture's own commit never runs it. + */ +function armRepo(sb: Sandbox): void { + git(sb, ['init', '-q', '-b', 'main']); + fs.writeFileSync(path.join(sb.repo, 'tracked.txt'), 'one\n'); + git(sb, ['add', 'tracked.txt']); + git(sb, ['commit', '-q', '-m', 'init']); + fs.writeFileSync(path.join(sb.repo, 'tracked.txt'), 'one\ntwo\n'); + fs.writeFileSync(path.join(sb.repo, 'untracked-probe.txt'), 'new\n'); + const hook = path.join(sb.base, 'fsmonitor-hook.sh'); + fs.writeFileSync(hook, `#!/bin/sh\necho ran >> '${sb.marker}'\nexit 1\n`); + fs.chmodSync(hook, 0o755); + git(sb, ['config', 'core.fsmonitor', hook]); +} + +/** Known-bad probe: an unguarded index read in the same repository runs the hook. */ +function expectUnguardedReadRunsHook(sb: Sandbox): void { + expect(fs.existsSync(sb.marker)).toBe(false); + git(sb, ['status', '--porcelain']); + expect(fs.existsSync(sb.marker), 'the fixture hook is live').toBe(true); +} + +describe('D-NO-FSMONITOR: index reads never run the repository\'s fsmonitor hook', () => { + let sb: Sandbox; + + beforeEach(() => { + sb = makeSandbox('df-fsmonitor'); + armRepo(sb); + }); + + afterEach(() => { + fs.rmSync(sb.base, { recursive: true, force: true }); + }); + + it('pre-compact-memory records git status and diff without running the hook', () => { + fs.mkdirSync(path.join(sb.repo, '.devflow', 'memory'), { recursive: true }); + const run = runHook(PRE_COMPACT_HOOK, { cwd: sb.repo, session_id: 'fsmonitor-test' }, sb.home, { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: '/dev/null', + }); + expect(run.exitCode, run.stderr).toBe(0); + + const backup = JSON.parse( + fs.readFileSync(path.join(sb.repo, '.devflow', 'memory', 'backup.json'), 'utf-8'), + ) as { git: { status: string; diff_stat: string } }; + expect(fs.existsSync(sb.marker), 'pre-compact-memory ran the fsmonitor hook').toBe(false); + expect(backup.git.status).toContain('untracked-probe.txt'); + expect(backup.git.diff_stat).toContain('tracked.txt'); + + expectUnguardedReadRunsHook(sb); + }, HOOK_TIMEOUT_MS); + + it('background-memory-update puts git status and diff in the prompt without running the hook', () => { + const memoryDir = path.join(sb.repo, '.devflow', 'memory'); + fs.mkdirSync(memoryDir, { recursive: true }); + const ts = Math.floor(Date.now() / 1000); + fs.writeFileSync( + path.join(memoryDir, '.pending-turns.jsonl'), + [ + JSON.stringify({ role: 'user', content: 'implement the feature', ts }), + JSON.stringify({ role: 'assistant', content: 'Sure, implementing now...', ts: ts + 1 }), + ].join('\n') + '\n', + ); + // A fake claude that captures the prompt and writes the staged memory file. + const shimDir = path.join(sb.base, 'shim'); + fs.mkdirSync(shimDir); + const captured = path.join(sb.base, 'prompt.txt'); + const staged = path.join(memoryDir, 'WORKING-MEMORY.md.new'); + fs.writeFileSync( + path.join(shimDir, 'claude'), + `#!/bin/bash\ncat > '${captured}'\necho "" > '${staged}'\nexit 0\n`, + ); + fs.chmodSync(path.join(shimDir, 'claude'), 0o755); + + execSync(`bash "${BACKGROUND_UPDATER}" "${sb.repo}"`, { + env: { ...sb.env, PATH: `${shimDir}:${process.env.PATH ?? '/usr/bin:/bin'}` }, + // The worker's watchdog inherits fds; 'ignore' keeps node from waiting on it. + stdio: 'ignore', + }); + + const prompt = fs.readFileSync(captured, 'utf-8'); + expect(fs.existsSync(sb.marker), 'background-memory-update ran the fsmonitor hook').toBe(false); + expect(prompt).toContain('untracked-probe.txt'); + expect(prompt).toMatch(/Diff summary:\n.*tracked\.txt/); + + expectUnguardedReadRunsHook(sb); + }, HOOK_TIMEOUT_MS); +}); From f484b4e5ae39eb838afcf0b2d868f4be165cdfeb Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 30 Sep 2026 23:15:43 +0300 Subject: [PATCH 2/4] test: guard every shipped git index read against core.fsmonitor A static guard over src/assets/scripts and src/**: each git call spelled with an index-reading subcommand (shell, command string, argv literal or git-named wrapper) must carry core.fsmonitor=false before the subcommand, or go through a wrapper that prepends it. Seeded probes pin every spelling red and prose, guarded calls and non-index subcommands green. Against the pre-fix tree it reports exactly the seven sites the previous commit guarded. --- tests/guards/no-fsmonitor-index-read.test.ts | 245 +++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 tests/guards/no-fsmonitor-index-read.test.ts diff --git a/tests/guards/no-fsmonitor-index-read.test.ts b/tests/guards/no-fsmonitor-index-read.test.ts new file mode 100644 index 000000000..2565a4bca --- /dev/null +++ b/tests/guards/no-fsmonitor-index-read.test.ts @@ -0,0 +1,245 @@ +/** + * no-fsmonitor-index-read — every git call in shipped code that reads the index + * turns the repository's `core.fsmonitor` command off for itself (D-NO-FSMONITOR). + * + * git runs the command a repository's config names in `core.fsmonitor` whenever + * it reads the index — `status`, `diff`, `ls-files` and every command that + * stages, commits or checks out. devflow's hooks, resolvers and HUD run inside + * arbitrary repositories, so an unguarded index read there executes code the + * repository chose. The override is `-c core.fsmonitor=false` before the + * subcommand. The behavioural tests (tests/memory-hooks-fsmonitor.test.ts, + * tests/decisions/ledger-ops.test.ts, tests/integration/hud-git.test.ts, + * tests/evidence-policy/settings-mode.test.ts) prove the override holds at run + * time; this guard stops a new call from forgetting it. + * + * What it asserts + * --------------- + * Across `src/assets/scripts/**` and every code file under `src/`, each git call + * spelled with an index-reading subcommand carries `core.fsmonitor=false` ahead + * of that subcommand, in one of three spellings: + * - shell, and a quoted JS command string: `git [global options] ` — + * a line that opens as a comment (`#` in shell; `*`, `//`, `/*` in JS) + * is prose and is skipped; + * - a JS argv literal handed to git: `'git', [ ..., ''` or a call to a + * git-named wrapper, `git(io, [ ..., ''` / `gitExec([ ..., ''`; + * - a call through a wrapper whose own body prepends the override + * (`'git', ['-c', 'core.fsmonitor=false', ...args]`), which covers every + * call made through that wrapper name in that file. + * + * What a green run does NOT prove (PF-064) + * ---------------------------------------- + * The matcher reads calls as they are spelled. An argv assembled at run time + * (`[sub, ...rest]`, an argv held in a named constant), a wrapper not named for + * git, and `describe --dirty` (the one index read `describe` makes, and never + * spelled in shipped code) are not matches. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import * as path from 'path'; + +import { ROOT, walkFiles, type CorpusEntry } from '../helpers.js'; + +// --------------------------------------------------------------------------- +// The rule +// --------------------------------------------------------------------------- + +/** git subcommands that read the index, and so run a configured fsmonitor. */ +const INDEX_READING_SUBCOMMANDS: readonly string[] = [ + 'status', 'diff', 'ls-files', 'add', 'commit', 'stash', 'update-index', 'checkout', + 'reset', 'grep', 'restore', 'switch', 'rm', 'mv', 'apply', 'merge', 'rebase', + 'cherry-pick', 'revert', 'am', +]; +const SUB = INDEX_READING_SUBCOMMANDS.join('|'); + +/** The override, as a shell/command-string global option. */ +const SHELL_OVERRIDE = /-c\s+core\.fsmonitor=false(?!\S)/; +/** The override, as an argv element. */ +const ARGV_OVERRIDE = /(['"])core\.fsmonitor=false\1/; + +/** + * `git`, its global options, then an index-reading subcommand, in a shell line + * or a quoted JS command string. Group 1: the global options. + */ +const SHELL_CALL = new RegExp( + String.raw`\bgit((?:\s+-[cC]\s+\S+|\s+--[\w-]+(?:=\S+)?)*)\s+(?:${SUB})(?![\w-])`, + 'g', +); +/** The same call opening a quoted JS command string (`execSync('git status')`). */ +const JS_COMMAND_STRING = new RegExp(String.raw`(['"\`])${SHELL_CALL.source}`, 'g'); + +/** + * A JS argv literal handed to git — `'git', [` or a git-named callee's argument + * list — whose leading string elements end in an index-reading subcommand. + * Groups: 1 the git-named callee (absent for `'git', [`), 2 the elements before + * the subcommand. + */ +const JS_ARGV_CALL = new RegExp( + String.raw`(?:(['"])git\1\s*,\s*|\b(\w*[gG]it\w*)\s*\(\s*(?:[\w.]+\s*,\s*)*)` + + String.raw`\[((?:\s*(['"])[^'"\n]*\4\s*,)*?)\s*(['"])(?:${SUB})\5`, + 'g', +); + +/** A function whose body spawns git with the override prepended to its argv. */ +const GUARDED_WRAPPER = /function\s+(\w+)\s*\([^)]*\)[^{]*\{(?:(?!\n\})[\s\S])*?(['"])git\2\s*,\s*\[\s*(['"])-c\3\s*,\s*(['"])core\.fsmonitor=false\4\s*,\s*\.\.\.\w+/g; + +interface UnguardedRead { + path: string; + line: number; + call: string; +} + +const SHELL_FILE = (p: string): boolean => path.extname(p) === '' || p.endsWith('.sh'); + +function lineAt(content: string, index: number): number { + return content.slice(0, index).split('\n').length; +} + +/** True when the match at `index` sits on a line that opens as a comment. */ +function onCommentLine(content: string, index: number, opener: RegExp): boolean { + const lineStart = content.lastIndexOf('\n', index - 1) + 1; + return opener.test(content.slice(lineStart, index)); +} + +const SHELL_COMMENT = /^\s*#/; +const JS_COMMENT = /^\s*(?:\*|\/\/|\/\*)/; + +/** Names of the wrappers in `content` that prepend the override themselves. */ +function guardedWrappers(content: string): Set { + return new Set([...content.matchAll(GUARDED_WRAPPER)].map(m => m[1])); +} + +/** + * Every index-reading git call in `corpus` without the override. Pure — the + * live arm and the seeded probes run this same function. + */ +function findUnguardedIndexReads(corpus: readonly CorpusEntry[]): UnguardedRead[] { + const found: UnguardedRead[] = []; + for (const entry of corpus) { + const { content } = entry; + const report = (index: number, call: string): void => { + found.push({ path: entry.path, line: lineAt(content, index), call: call.trim().split('\n')[0] }); + }; + if (SHELL_FILE(entry.path)) { + for (const m of content.matchAll(SHELL_CALL)) { + if (onCommentLine(content, m.index, SHELL_COMMENT)) continue; + if (!SHELL_OVERRIDE.test(m[1])) report(m.index, m[0]); + } + continue; + } + for (const m of content.matchAll(JS_COMMAND_STRING)) { + if (onCommentLine(content, m.index, JS_COMMENT)) continue; + if (!SHELL_OVERRIDE.test(m[2])) report(m.index, m[0]); + } + const wrappers = guardedWrappers(content); + for (const m of content.matchAll(JS_ARGV_CALL)) { + const callee = m[2]; + if (callee !== undefined && wrappers.has(callee)) continue; + if (!ARGV_OVERRIDE.test(m[3])) report(m.index, m[0]); + } + } + return found; +} + +/** Every index-reading git call in `corpus`, guarded or not — the non-vacuity count. */ +function countIndexReads(corpus: readonly CorpusEntry[]): number { + let n = 0; + for (const { path: p, content } of corpus) { + n += SHELL_FILE(p) + ? [...content.matchAll(SHELL_CALL)].length + : [...content.matchAll(JS_COMMAND_STRING)].length + [...content.matchAll(JS_ARGV_CALL)].length; + } + return n; +} + +// --------------------------------------------------------------------------- +// Corpus +// --------------------------------------------------------------------------- + +const CODE_EXTENSIONS: readonly string[] = ['.ts', '.mts', '.cts', '.js', '.mjs', '.cjs']; +const SCRIPTS_DIR = path.join(ROOT, 'src', 'assets', 'scripts'); + +const corpus: CorpusEntry[] = walkFiles( + path.join(ROOT, 'src'), + file => file.startsWith(SCRIPTS_DIR + path.sep) + ? CODE_EXTENSIONS.includes(path.extname(file)) || SHELL_FILE(file) + : CODE_EXTENSIONS.includes(path.extname(file)), +).map(file => ({ + path: path.relative(ROOT, file).split(path.sep).join('/'), + content: readFileSync(file, 'utf-8'), +})); + +// --------------------------------------------------------------------------- +// Live arms +// --------------------------------------------------------------------------- + +describe('every index-reading git call turns core.fsmonitor off (D-NO-FSMONITOR)', () => { + it('reaches the shipped scripts, the HUD, and their index reads', () => { + const paths = corpus.map(e => e.path); + expect(paths).toContain('src/assets/scripts/hooks/json-helper.cjs'); + expect(paths).toContain('src/assets/scripts/hooks/pre-compact-memory'); + expect(paths).toContain('src/hud/git.ts'); + // json-helper ls-files, resolve-settings ls-files, verify-evidence diff, + // two each in pre-compact-memory and background-memory-update, HUD status + diff. + expect(countIndexReads(corpus)).toBeGreaterThanOrEqual(9); + }); + + it('no git call reads the index without -c core.fsmonitor=false', () => { + expect( + findUnguardedIndexReads(corpus).map(o => `${o.path}:${o.line}: \`${o.call}\` — add -c core.fsmonitor=false before the subcommand`), + ).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// Seeded probes (PF-064): red on every spelling, green on prose and guards. +// --------------------------------------------------------------------------- + +describe('no-fsmonitor-index-read guard: seeded probes', () => { + const SH = 'src/assets/scripts/hooks/example-hook'; + const JS = 'src/assets/scripts/example.cjs'; + + const KNOWN_BAD: ReadonlyArray<{ name: string; entry: CorpusEntry }> = [ + { name: 'a shell status', entry: { path: SH, content: 'S=$(git status --short 2>/dev/null)\n' } }, + { name: 'a shell diff after -C', entry: { path: SH, content: 'git -C "$d" diff --stat HEAD\n' } }, + { name: 'a shell ls-files with another -c', entry: { path: SH, content: 'git -c color.ui=never ls-files -z\n' } }, + { name: 'the override after the subcommand', entry: { path: SH, content: 'git status -c core.fsmonitor=false\n' } }, + { name: "a 'git', [...] argv", entry: { path: JS, content: "execFileSync('git', ['ls-files', '-z'], {});" } }, + { name: 'an argv with only other global options', entry: { path: JS, content: "execFile('git', ['--no-optional-locks', 'status'], cb);" } }, + { name: 'a git-named wrapper call', entry: { path: 'src/hud/x.ts', content: "await gitExec(['diff', '--shortstat', base], cwd);" } }, + { name: 'a wrapper call across lines', entry: { path: JS, content: "runGit(exec, root,\n ['add', '--', file], MAX);" } }, + { name: 'a command string', entry: { path: 'src/core/x.ts', content: "execSync('git commit -m x');" } }, + { + name: 'a call through an unguarded wrapper', + entry: { path: JS, content: "function git(io, args) {\n return run(io, 'git', [...args]);\n}\ngit(io, ['status']);" }, + }, + ]; + + for (const probe of KNOWN_BAD) { + it(`reports ${probe.name}`, () => { + expect(findUnguardedIndexReads([probe.entry])).toHaveLength(1); + }); + } + + it('names the line the call starts on', () => { + const found = findUnguardedIndexReads([{ path: JS, content: "// header\n\nexecFileSync('git',\n ['status']);" }]); + expect(found.map(f => f.line)).toEqual([3]); + }); + + it('does not report guarded calls, non-index subcommands, or prose', () => { + const clean: CorpusEntry[] = [ + { path: SH, content: 'S=$(git -c core.fsmonitor=false status --short)\n# git status is guarded above\n' }, + { path: SH, content: 'git rev-parse HEAD; git log --oneline -5; git merge-base --is-ancestor a b\n' }, + { path: SH, content: 'echo "not a git repo"\ndbg "git state captured"\n' }, + { path: JS, content: "execFileSync('git', ['-c', 'core.fsmonitor=false', 'ls-files', '-z']);" }, + { path: JS, content: "runGit(exec, root, ['-c', 'core.fsmonitor=false', 'ls-files', '--error-unmatch'], MAX);" }, + { + path: JS, + content: "function git(io, args) {\n return runCall(io, 'git', ['-c', 'core.fsmonitor=false', ...args]);\n}\ngit(io, ['diff', '--name-only']);", + }, + { path: JS, content: "/** List tracked files via `git ls-files`. */\nexecFile('git', ['rev-parse', '--show-toplevel']);" }, + { path: 'src/cli/x.ts', content: "const ACTIONS = ['enable', 'disable', 'status'];\nexecFileSync('sudo', ['rm', p]);" }, + ]; + expect(findUnguardedIndexReads(clean)).toEqual([]); + }); +}); From b4def9e3f5d43ec52709cb9715432c7e26602cf8 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 30 Sep 2026 23:49:24 +0300 Subject: [PATCH 3/4] fix(hud): keep git's built-in fsmonitor daemon, block only repo hooks The HUD passed -c core.fsmonitor=false on every call, which also turned off git's built-in fsmonitor daemon. That daemon is git's own code, not a command from the repository, and it is what keeps status fast in huge repositories, where the HUD's 1s timeout would otherwise expire and draw a clean tree. Each refresh now reads core.fsmonitor once with git config --type=bool --get (no index read, never cached). Only the literal answer true lets status and diff run without the override. A hook path (refused by --type=bool), any other value, an unset key and a failed read keep it (fail closed). Every other HUD call keeps the override unconditionally. The static guard honours the conditional wrapper only in src/hud/git.ts, only while that file defines fsmonitorOverride itself, and runs the classifier to require it to fail closed for every answer but true. --- src/hud/git.ts | 59 +++++-- tests/guards/no-fsmonitor-index-read.test.ts | 73 ++++++++- tests/hud-git-fsmonitor.test.ts | 155 +++++++++++++++++++ tests/integration/hud-git.test.ts | 7 +- 4 files changed, 277 insertions(+), 17 deletions(-) create mode 100644 tests/hud-git-fsmonitor.test.ts diff --git a/src/hud/git.ts b/src/hud/git.ts index d8bfd6613..a24f5b393 100644 --- a/src/hud/git.ts +++ b/src/hud/git.ts @@ -14,18 +14,51 @@ function shellExec(cmd: string, args: string[], cwd: string): Promise { } /** - * Every HUD git call: `-c core.fsmonitor=false` first. + * The override that stops git running a repository's `core.fsmonitor` command. * - * D-NO-FSMONITOR: `status` and `diff` read the index, and reading the index - * runs the command a repository's config names in `core.fsmonitor` — code - * chosen by whatever repository the status line is drawn in, on every prompt. - * The override keeps each call a pure read; calls that never touch the index - * carry it too, so no new call can forget it. + * D-NO-FSMONITOR: git runs the command a repository's config names in + * `core.fsmonitor` whenever it reads the index — code chosen by whatever + * repository the status line is drawn in, on every prompt. Every HUD git call + * carries this override (`gitExec`), except the index reads' carve-out below. + */ +const FSMONITOR_OFF: readonly string[] = ['-c', 'core.fsmonitor=false']; + +/** `core.fsmonitor` as git itself reads it, without touching the index. */ +const FSMONITOR_CONFIG_READ: readonly string[] = ['config', '--type=bool', '--get', 'core.fsmonitor']; + +/** + * Every HUD git call except the index reads: the override first, so no call + * that never needs fsmonitor can run a repository's hook. */ function gitExec(args: string[], cwd: string): Promise { return shellExec('git', ['-c', 'core.fsmonitor=false', ...args], cwd); } +/** + * D-NO-FSMONITOR carve-out: the override for an index read, given the + * trimmed stdout of `git config --type=bool --get core.fsmonitor`. + * + * `true` is the built-in fsmonitor daemon — git's own code, not a command from + * the repository — and it is what keeps `status` fast in huge repositories, + * where the HUD's 1s timeout would otherwise expire and draw a clean tree. Only + * that exact answer drops the override. A hook path (which `--type=bool` + * refuses), any other value, an unset key and a failed read all come back as + * something else, and keep it: the carve-out fails closed. + * + * @param fsmonitorConfig - the config read's trimmed stdout; '' when it failed + */ +export function fsmonitorOverride(fsmonitorConfig: string): readonly string[] { + return fsmonitorConfig === 'true' ? [] : FSMONITOR_OFF; +} + +/** + * The HUD's index reads (`status`, `diff`): the override unless this refresh's + * `core.fsmonitor` read named the built-in daemon (`fsmonitorOverride`). + */ +function gitIndexRead(args: string[], cwd: string, fsmonitorConfig: string): Promise { + return shellExec('git', [...fsmonitorOverride(fsmonitorConfig), ...args], cwd); +} + /** * Gather git status for the given working directory. * Returns null if not in a git repo or on error. @@ -35,8 +68,13 @@ export async function gatherGitStatus(cwd: string): Promise { const topLevel = await gitExec(['rev-parse', '--show-toplevel'], cwd); if (!topLevel) return null; - // Branch name — 'HEAD' means detached HEAD state - const branch = await gitExec(['rev-parse', '--abbrev-ref', 'HEAD'], cwd); + // Branch name — 'HEAD' means detached HEAD state. core.fsmonitor is read once + // per refresh, never cached (the setting can change between prompts), and + // without the override, which would answer for it. + const [branch, fsmonitorConfig] = await Promise.all([ + gitExec(['rev-parse', '--abbrev-ref', 'HEAD'], cwd), + shellExec('git', [...FSMONITOR_CONFIG_READ], cwd), + ]); if (!branch) return null; // Dirty check — porcelain v1: two-char XY status prefix per path. @@ -44,9 +82,10 @@ export async function gatherGitStatus(cwd: string): Promise { // `git status --no-optional-locks` is rejected as an unknown option, which makes // shellExec return '' and silently reports every tree as clean. Keeping the flag // (in the right position) stops the HUD from writing .git/index on every prompt. - const statusOutput = await gitExec( + const statusOutput = await gitIndexRead( ['--no-optional-locks', 'status', '--porcelain'], cwd, + fsmonitorConfig, ); let dirty = false; let staged = false; @@ -89,7 +128,7 @@ export async function gatherGitStatus(cwd: string): Promise { // NOTE: diff includes the working tree; ahead/behind counts commits only. This asymmetry is // deliberate — both reference the same merge base but differ in working-tree inclusion. if (mergeBase) { - const diffStat = await gitExec(['diff', '--shortstat', mergeBase], cwd); + const diffStat = await gitIndexRead(['diff', '--shortstat', mergeBase], cwd, fsmonitorConfig); const filesMatch = diffStat.match(/(\d+)\s+file/); const addMatch = diffStat.match(/(\d+)\s+insertion/); const delMatch = diffStat.match(/(\d+)\s+deletion/); diff --git a/tests/guards/no-fsmonitor-index-read.test.ts b/tests/guards/no-fsmonitor-index-read.test.ts index 2565a4bca..2f7427cdb 100644 --- a/tests/guards/no-fsmonitor-index-read.test.ts +++ b/tests/guards/no-fsmonitor-index-read.test.ts @@ -24,7 +24,16 @@ * git-named wrapper, `git(io, [ ..., ''` / `gitExec([ ..., ''`; * - a call through a wrapper whose own body prepends the override * (`'git', ['-c', 'core.fsmonitor=false', ...args]`), which covers every - * call made through that wrapper name in that file. + * call made through that wrapper name in that file; + * - the one audited carve-out: a wrapper in `src/hud/git.ts` whose body + * spawns `'git', [...fsmonitorOverride(x), ...args]`. It drops the override + * only when `core.fsmonitor` is the built-in daemon (git's own code, not a + * repository command). The carve-out is closed three ways: it is honoured + * in that file alone (the same spelling anywhere else is unguarded), the + * file must define `fsmonitorOverride` itself rather than import one, and + * the live arm runs that function and requires it to keep the override for + * every answer but the literal `true`. Its argv behaviour per setting is + * pinned in tests/hud-git-fsmonitor.test.ts. * * What a green run does NOT prove (PF-064) * ---------------------------------------- @@ -39,6 +48,7 @@ import { readFileSync } from 'fs'; import * as path from 'path'; import { ROOT, walkFiles, type CorpusEntry } from '../helpers.js'; +import { fsmonitorOverride } from '../../src/hud/git.js'; // --------------------------------------------------------------------------- // The rule @@ -81,7 +91,14 @@ const JS_ARGV_CALL = new RegExp( ); /** A function whose body spawns git with the override prepended to its argv. */ -const GUARDED_WRAPPER = /function\s+(\w+)\s*\([^)]*\)[^{]*\{(?:(?!\n\})[\s\S])*?(['"])git\2\s*,\s*\[\s*(['"])-c\3\s*,\s*(['"])core\.fsmonitor=false\4\s*,\s*\.\.\.\w+/g; +const GUARDED_WRAPPER = /function\s+(\w+)\s*\([^)]*\)[^{]*\{(?:(?!\n\}|\bfunction\b)[\s\S])*?(['"])git\2\s*,\s*\[\s*(['"])-c\3\s*,\s*(['"])core\.fsmonitor=false\4\s*,\s*\.\.\.\w+/g; + +/** A wrapper that spawns git with the carve-out classifier's answer prepended. */ +const CARVE_OUT_WRAPPER = /function\s+(\w+)\s*\([^)]*\)[^{]*\{(?:(?!\n\}|\bfunction\b)[\s\S])*?(['"])git\2\s*,\s*\[\s*\.\.\.fsmonitorOverride\(\s*\w+\s*\)\s*,\s*\.\.\.\w+\s*\]/g; + +/** The one file whose carve-out wrapper is honoured, and the classifier it must define. */ +const CARVE_OUT_FILE = 'src/hud/git.ts'; +const CARVE_OUT_CLASSIFIER_DEFINITION = /\bexport\s+function\s+fsmonitorOverride\s*\(/; interface UnguardedRead { path: string; @@ -104,9 +121,17 @@ function onCommentLine(content: string, index: number, opener: RegExp): boolean const SHELL_COMMENT = /^\s*#/; const JS_COMMENT = /^\s*(?:\*|\/\/|\/\*)/; -/** Names of the wrappers in `content` that prepend the override themselves. */ -function guardedWrappers(content: string): Set { - return new Set([...content.matchAll(GUARDED_WRAPPER)].map(m => m[1])); +/** + * Names of the wrappers in `entry` that guard their calls: those that prepend + * the override, and — in the carve-out file only, and only while it defines the + * classifier itself — those that prepend the classifier's answer. + */ +function guardedWrappers(entry: CorpusEntry): Set { + const names = [...entry.content.matchAll(GUARDED_WRAPPER)].map(m => m[1]); + if (entry.path === CARVE_OUT_FILE && CARVE_OUT_CLASSIFIER_DEFINITION.test(entry.content)) { + names.push(...[...entry.content.matchAll(CARVE_OUT_WRAPPER)].map(m => m[1])); + } + return new Set(names); } /** @@ -131,7 +156,7 @@ function findUnguardedIndexReads(corpus: readonly CorpusEntry[]): UnguardedRead[ if (onCommentLine(content, m.index, JS_COMMENT)) continue; if (!SHELL_OVERRIDE.test(m[2])) report(m.index, m[0]); } - const wrappers = guardedWrappers(content); + const wrappers = guardedWrappers(entry); for (const m of content.matchAll(JS_ARGV_CALL)) { const callee = m[2]; if (callee !== undefined && wrappers.has(callee)) continue; @@ -189,6 +214,16 @@ describe('every index-reading git call turns core.fsmonitor off (D-NO-FSMONITOR) findUnguardedIndexReads(corpus).map(o => `${o.path}:${o.line}: \`${o.call}\` — add -c core.fsmonitor=false before the subcommand`), ).toEqual([]); }); + + it('the carve-out file defines its classifier, which keeps the override for every answer but `true`', () => { + const hud = corpus.find(e => e.path === CARVE_OUT_FILE); + expect(hud?.content).toMatch(CARVE_OUT_CLASSIFIER_DEFINITION); + expect([...(hud?.content ?? '').matchAll(CARVE_OUT_WRAPPER)].length, 'the carve-out wrapper is live').toBe(1); + expect(fsmonitorOverride('true')).toEqual([]); + for (const answer of ['', 'false', 'TRUE', 'yes', '1', ' true', 'true\n', '/tmp/hook.sh']) { + expect(fsmonitorOverride(answer), JSON.stringify(answer)).toEqual(['-c', 'core.fsmonitor=false']); + } + }); }); // --------------------------------------------------------------------------- @@ -242,4 +277,30 @@ describe('no-fsmonitor-index-read guard: seeded probes', () => { ]; expect(findUnguardedIndexReads(clean)).toEqual([]); }); + + const CARVE_OUT = [ + 'export function fsmonitorOverride(v: string): readonly string[] { return v === \'true\' ? [] : OFF; }', + 'function gitIndexRead(args: string[], cwd: string, cfg: string) {', + ' return shellExec(\'git\', [...fsmonitorOverride(cfg), ...args], cwd);', + '}', + "gitIndexRead(['--no-optional-locks', 'status', '--porcelain'], cwd, cfg);", + ].join('\n'); + + it('honours the carve-out wrapper in its own file', () => { + expect(findUnguardedIndexReads([{ path: CARVE_OUT_FILE, content: CARVE_OUT }])).toEqual([]); + }); + + it('reports the carve-out spelling in any other file', () => { + expect(findUnguardedIndexReads([{ path: 'src/core/elsewhere.ts', content: CARVE_OUT }])).toHaveLength(1); + }); + + it('reports the carve-out wrapper when its file imports the classifier instead of defining it', () => { + const imported = CARVE_OUT.replace(/^export function fsmonitorOverride[^\n]*\n/, "import { fsmonitorOverride } from './x.js';\n"); + expect(findUnguardedIndexReads([{ path: CARVE_OUT_FILE, content: imported }])).toHaveLength(1); + }); + + it('reports a wrapper that prepends some other function\'s answer', () => { + const other = CARVE_OUT.replace('[...fsmonitorOverride(cfg), ...args]', '[...maybeOverride(cfg), ...args]'); + expect(findUnguardedIndexReads([{ path: CARVE_OUT_FILE, content: other }])).toHaveLength(1); + }); }); diff --git a/tests/hud-git-fsmonitor.test.ts b/tests/hud-git-fsmonitor.test.ts new file mode 100644 index 000000000..df8d941c7 --- /dev/null +++ b/tests/hud-git-fsmonitor.test.ts @@ -0,0 +1,155 @@ +/** + * The HUD's fsmonitor carve-out (D-NO-FSMONITOR in src/hud/git.ts), pinned at + * the argv level through a scripted execFile — no real git, no real daemon. + * + * Before its index reads (`status`, `diff`) the HUD reads `core.fsmonitor` once + * per refresh with `git config --type=bool --get`, which never touches the + * index. Only a value git reads as boolean true — the built-in daemon, git's + * own code — lets the index reads run without `-c core.fsmonitor=false`. A hook + * path, any other value, an unset key and a failed read all fail closed to the + * override. Every other HUD git call carries the override unconditionally. + * + * A repository whose `core.fsmonitor` names a real hook script is covered with + * real git in tests/integration/hud-git.test.ts. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +/** What the fake git answers for one call: stdout, or a failure. */ +type FakeAnswer = { stdout: string } | { fail: string }; + +/** How the fake answers the `core.fsmonitor` read, per scenario. */ +let fsmonitorAnswers: FakeAnswer[] = []; + +/** argv of every git call, in order. */ +const calls: string[][] = []; + +const OVERRIDE = ['-c', 'core.fsmonitor=false']; +const CONFIG_READ = ['config', '--type=bool', '--get', 'core.fsmonitor']; + +/** The subcommand argv with a leading override removed. */ +function bare(args: readonly string[]): string[] { + return args[0] === OVERRIDE[0] && args[1] === OVERRIDE[1] ? args.slice(2) : [...args]; +} + +/** A repository on `feature` with one unstaged edit, compared against local `main`. */ +function answer(args: readonly string[]): FakeAnswer { + const a = bare(args); + const sub = a.find(x => !x.startsWith('-')) ?? ''; + if (a.join(' ') === CONFIG_READ.join(' ')) { + return fsmonitorAnswers.shift() ?? { fail: 'unexpected extra config read' }; + } + switch (sub) { + case 'rev-parse': + return a.includes('--show-toplevel') ? { stdout: '/repo\n' } : { stdout: 'feature\n' }; + case 'status': + return { stdout: '?? untracked.txt\n' }; + case 'symbolic-ref': + return { fail: 'no origin/HEAD' }; + case 'for-each-ref': + return { stdout: 'feature\nmain\n' }; + case 'rev-list': + return { stdout: '0\t1\n' }; + case 'merge-base': + return { stdout: 'abc123\n' }; + case 'diff': + return { stdout: ' 1 file changed, 1 insertion(+)\n' }; + case 'describe': + return { fail: 'no tags' }; + case 'worktree': + return { stdout: '/repo abc123 [feature]\n' }; + default: + return { fail: `unscripted: ${a.join(' ')}` }; + } +} + +vi.mock('node:child_process', async (importOriginal) => ({ + ...(await importOriginal()), + execFile: vi.fn((file: string, args: string[], _opts: object, cb: (err: unknown, stdout: string) => void) => { + expect(file).toBe('git'); + calls.push([...args]); + const res = answer(args); + // Async like the real execFile, so Promise.all ordering stays realistic. + setImmediate(() => ('fail' in res + ? cb(Object.assign(new Error(res.fail), { code: 1 }), '') + : cb(null, res.stdout))); + }), +})); + +import { gatherGitStatus, fsmonitorOverride } from '../src/hud/git.js'; + +/** argv of the calls whose subcommand is `sub`. */ +function callsTo(sub: string): string[][] { + return calls.filter(args => bare(args).find(x => !x.startsWith('-')) === sub); +} + +describe('HUD fsmonitor carve-out (D-NO-FSMONITOR)', () => { + beforeEach(() => { + calls.length = 0; + fsmonitorAnswers = []; + }); + + it('reads core.fsmonitor once per refresh, without the override that would answer for it', async () => { + fsmonitorAnswers = [{ stdout: 'true\n' }]; + await gatherGitStatus('/repo'); + expect(calls.filter(args => bare(args).join(' ') === CONFIG_READ.join(' '))).toEqual([CONFIG_READ]); + }); + + it('core.fsmonitor=true (the built-in daemon): status and diff run without the override', async () => { + fsmonitorAnswers = [{ stdout: 'true\n' }]; + const status = await gatherGitStatus('/repo'); + expect(callsTo('status')).toEqual([['--no-optional-locks', 'status', '--porcelain']]); + expect(callsTo('diff')).toEqual([['diff', '--shortstat', 'abc123']]); + expect(status?.dirty).toBe(true); + expect(status?.filesChanged).toBe(1); + }); + + const FAIL_CLOSED: ReadonlyArray<{ name: string; read: FakeAnswer }> = [ + { name: 'unset (git config exits 1, prints nothing)', read: { fail: 'exit 1' } }, + { name: 'a hook path (--type=bool refuses it, exit 128)', read: { fail: 'bad boolean config value' } }, + { name: 'false', read: { stdout: 'false\n' } }, + { name: 'a failed read (timeout)', read: { fail: 'ETIMEDOUT' } }, + ]; + + for (const { name, read } of FAIL_CLOSED) { + it(`core.fsmonitor ${name}: status and diff carry the override`, async () => { + fsmonitorAnswers = [read]; + await gatherGitStatus('/repo'); + expect(callsTo('status')).toEqual([[...OVERRIDE, '--no-optional-locks', 'status', '--porcelain']]); + expect(callsTo('diff')).toEqual([[...OVERRIDE, 'diff', '--shortstat', 'abc123']]); + }); + } + + it('re-reads the setting on every refresh: no cached carve-out outlives a config change', async () => { + fsmonitorAnswers = [{ stdout: 'true\n' }, { fail: 'bad boolean config value' }]; + await gatherGitStatus('/repo'); + await gatherGitStatus('/repo'); + expect(callsTo('status')).toEqual([ + ['--no-optional-locks', 'status', '--porcelain'], + [...OVERRIDE, '--no-optional-locks', 'status', '--porcelain'], + ]); + }); + + it('every call other than the config read carries the override whatever the setting', async () => { + fsmonitorAnswers = [{ stdout: 'true\n' }]; + await gatherGitStatus('/repo'); + const others = calls.filter(args => { + const sub = bare(args).find(x => !x.startsWith('-')); + return sub !== 'config' && sub !== 'status' && sub !== 'diff'; + }); + expect(others.length).toBeGreaterThan(0); + for (const args of others) expect(args.slice(0, 2)).toEqual(OVERRIDE); + }); +}); + +describe('fsmonitorOverride — the classifier the carve-out rests on', () => { + it('drops the override only for the literal boolean true git config prints', () => { + expect(fsmonitorOverride('true')).toEqual([]); + }); + + it('keeps the override for everything else', () => { + for (const value of ['', 'false', 'TRUE', 'yes', '1', ' true', 'true\n', '/tmp/hook.sh', 'true; rm']) { + expect(fsmonitorOverride(value), JSON.stringify(value)).toEqual(OVERRIDE); + } + }); +}); diff --git a/tests/integration/hud-git.test.ts b/tests/integration/hud-git.test.ts index 801915fe0..56bf46480 100644 --- a/tests/integration/hud-git.test.ts +++ b/tests/integration/hud-git.test.ts @@ -819,7 +819,12 @@ describe('gatherGitStatus — dirty-tree ahead/filesChanged asymmetry (Shape M)' .filter(args => args.includes('status')); expect(statusCalls).toHaveLength(1); - expect(statusCalls[0]).toEqual(['-c', 'core.fsmonitor=false', '--no-optional-locks', 'status', '--porcelain']); + // Ahead of the flag sits only the fsmonitor override, or nothing when the + // machine's core.fsmonitor is the built-in daemon (D-NO-FSMONITOR carve-out, + // pinned per setting in tests/hud-git-fsmonitor.test.ts). + const flagAt = statusCalls[0].indexOf('--no-optional-locks'); + expect([[], ['-c', 'core.fsmonitor=false']]).toContainEqual(statusCalls[0].slice(0, flagAt)); + expect(statusCalls[0].slice(flagAt)).toEqual(['--no-optional-locks', 'status', '--porcelain']); // The flag must not break the command: a rejected option would yield '' → clean. expect(live?.dirty).toBe(true); expect(live?.staged).toBe(true); From f98bcc558ecc4c3f6619bac89415ce52a707a877 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Wed, 30 Sep 2026 23:51:48 +0300 Subject: [PATCH 4/4] fix(hud): keep porcelain status's leading column when trimming shellExec trimmed every call's whole stdout, which also stripped the leading blank of the first `git status --porcelain` line. An unstaged edit prints ` M path`; trimmed, `M` moved into the index column, so a tree whose only change was an unstaged edit to a tracked file read as staged and clean. shellExec now takes a trim mode: whole-output trim stays the default for single values (refs, counts, the config answer), and the status call trims trailing whitespace only. --- src/hud/git.ts | 23 ++++++++++++++++++---- tests/integration/hud-git.test.ts | 32 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/hud/git.ts b/src/hud/git.ts index a24f5b393..653f029bf 100644 --- a/src/hud/git.ts +++ b/src/hud/git.ts @@ -5,10 +5,19 @@ import type { GitStatus } from './types.js'; const GIT_TIMEOUT = 1000; // 1s per command const GIT_MAXBUFFER = 16 * 1024 * 1024; // 16 MiB — covers >500k refs at ~30 B/ref -function shellExec(cmd: string, args: string[], cwd: string): Promise { +/** + * How a call's stdout is cleaned. `both` trims the whole output — right for a + * single value (a ref, a count, a config answer). `trailing` keeps leading + * whitespace, for `status --porcelain`, whose first column is data: an unstaged + * edit prints ` M path`, and a full trim would shift `M` into the index column. + */ +type StdoutTrim = 'both' | 'trailing'; + +function shellExec(cmd: string, args: string[], cwd: string, trim: StdoutTrim = 'both'): Promise { return new Promise((resolve) => { execFile(cmd, args, { cwd, timeout: GIT_TIMEOUT, maxBuffer: GIT_MAXBUFFER }, (err, stdout) => { - resolve(err ? '' : stdout.trim()); + if (err) return resolve(''); + resolve(trim === 'trailing' ? stdout.trimEnd() : stdout.trim()); }); }); } @@ -55,8 +64,13 @@ export function fsmonitorOverride(fsmonitorConfig: string): readonly string[] { * The HUD's index reads (`status`, `diff`): the override unless this refresh's * `core.fsmonitor` read named the built-in daemon (`fsmonitorOverride`). */ -function gitIndexRead(args: string[], cwd: string, fsmonitorConfig: string): Promise { - return shellExec('git', [...fsmonitorOverride(fsmonitorConfig), ...args], cwd); +function gitIndexRead( + args: string[], + cwd: string, + fsmonitorConfig: string, + trim: StdoutTrim = 'both', +): Promise { + return shellExec('git', [...fsmonitorOverride(fsmonitorConfig), ...args], cwd, trim); } /** @@ -86,6 +100,7 @@ export async function gatherGitStatus(cwd: string): Promise { ['--no-optional-locks', 'status', '--porcelain'], cwd, fsmonitorConfig, + 'trailing', ); let dirty = false; let staged = false; diff --git a/tests/integration/hud-git.test.ts b/tests/integration/hud-git.test.ts index 56bf46480..8a087eaa0 100644 --- a/tests/integration/hud-git.test.ts +++ b/tests/integration/hud-git.test.ts @@ -972,3 +972,35 @@ describe('gatherGitStatus — never runs the repository\'s core.fsmonitor hook ( } }); }); + +describe('gatherGitStatus — porcelain status keeps its leading column', () => { + /** + * `git status --porcelain` prints an unstaged edit as ` M path` — a blank index + * column, then the worktree column. Trimming the whole output removes the first + * line's leading blank, shifting `M` into the index column: an unstaged-only + * edit then reads as staged and the tree as clean. + */ + it('an unstaged edit to a tracked file alone reads dirty and unstaged, not staged', async () => { + const base = mkdtempSync(join(tmpdir(), 'hud-porcelain-')); + const repo = join(base, 'repo'); + try { + mkdirSync(repo); + git(repo, ['init', '-q', '-b', 'main']); + writeFileSync(join(repo, 'tracked.txt'), 'one\n'); + git(repo, ['add', 'tracked.txt']); + git(repo, ['commit', '-q', '-m', 'init']); + writeFileSync(join(repo, 'tracked.txt'), 'one\ntwo\n'); + + vi.stubEnv('HOME', join(base, 'home')); + vi.stubEnv('GIT_CONFIG_NOSYSTEM', '1'); + vi.stubEnv('GIT_CONFIG_GLOBAL', '/dev/null'); + const status = await gatherGitStatus(repo); + + expect(status?.dirty).toBe(true); + expect(status?.staged).toBe(false); + } finally { + vi.unstubAllEnvs(); + rmSync(base, { recursive: true, force: true }); + } + }); +});