From b447d6f5e18ad0e093c34a8f4881d4f0e5417820 Mon Sep 17 00:00:00 2001 From: Yudhi Armyndharis Date: Wed, 12 Aug 2026 16:14:27 +0700 Subject: [PATCH] fix: report what the repair took, and fold case on root instruction files Automatic repair restores staged deletions it cannot rule out as the source of a rename Git did not detect. That sweep is deliberate and stays. What was missing is that it never said so: the summary named only the blocked paths, and when the index ended up empty it claimed nothing else had been staged. A developer who had run `git rm --cached legacy.txt` got a clean `git status` afterwards, the file still tracked, and a commit message describing an untracking the commit did not carry. The repair now returns what it took beyond the blocked paths, names those paths with the reason, and gives the command to stage the removal again. Two tests asserted the old claim on runs that did have collateral; they now assert what the run actually did. Root instruction files also matched case-sensitively while every sibling path rule folds case. On APFS and NTFS `claude.md` is the same file as `CLAUDE.md`, so the guarded spelling was one keystroke from being skipped. The root patterns now carry case classes. Paths deeper in the tree stay case-sensitive, where these names are ordinary documentation. --- bin/aimhooman.mjs | 17 +++++++++++++-- rules/paths.json | 10 ++++----- src/guard.mjs | 11 ++++++++-- tests/cli.test.mjs | 41 +++++++++++++++++++++++++++++++---- tests/guard.test.mjs | 5 +++-- tests/rule-contracts.test.mjs | 17 ++++++++++----- 6 files changed, 80 insertions(+), 21 deletions(-) diff --git a/bin/aimhooman.mjs b/bin/aimhooman.mjs index 5ee7919..c1c9908 100644 --- a/bin/aimhooman.mjs +++ b/bin/aimhooman.mjs @@ -356,10 +356,23 @@ function cmdPrecommit(args) { const paths = [...new Set(blocks.map((f) => f.path).filter(Boolean))]; let emptied = false; try { - emptied = repairStagedBlocks(repo, blocks, paths); + const repair = repairStagedBlocks(repo, blocks, paths); + emptied = repair.emptied; + const stopped = emptied + ? (repair.collateral.length + ? ' — the index is empty now, so the commit is stopped rather than left empty' + : ' — nothing else was staged, so the commit is stopped rather than left empty') + : ''; process.stderr.write( - `aimhooman: unstaged ${paths.length} file(s) from this commit: ${paths.map(visible).join(', ')} (index only; nothing on disk was deleted)${emptied ? ' — nothing else was staged, so the commit is stopped rather than left empty' : ''}\n` + `aimhooman: unstaged ${paths.length} file(s) from this commit: ${paths.map(visible).join(', ')} (index only; nothing on disk was deleted)${stopped}\n` ); + if (repair.collateral.length) { + process.stderr.write( + `aimhooman: also restored ${repair.collateral.length} staged deletion(s) that could be the source of a rename Git cannot detect: ` + + `${repair.collateral.map(visible).join(', ')} — stage the removal again with 'git rm --cached ', ` + + "or 'git rm ' if you deleted it on disk too\n" + ); + } } catch (e) { process.stderr.write( `aimhooman: could not unstage protected files: ${e.message} ` + diff --git a/rules/paths.json b/rules/paths.json index f0fd69e..6e52aaf 100644 --- a/rules/paths.json +++ b/rules/paths.json @@ -218,20 +218,20 @@ }, { "id": "generic.agent-instructions", - "version": 2, + "version": 3, "provider": "generic", "category": "ambiguous-instructions", "confidence": "medium", "kind": "path", "match": { "paths": [ - "AGENTS.md", + "[Aa][Gg][Ee][Nn][Tt][Ss].[Mm][Dd]", "**/AGENTS.md", - "CLAUDE.md", + "[Cc][Ll][Aa][Uu][Dd][Ee].[Mm][Dd]", "**/CLAUDE.md", - "GEMINI.md", + "[Gg][Ee][Mm][Ii][Nn][Ii].[Mm][Dd]", "**/GEMINI.md", - ".github/copilot-instructions.md", + ".github/[Cc][Oo][Pp][Ii][Ll][Oo][Tt]-[Ii][Nn][Ss][Tt][Rr][Uu][Cc][Tt][Ii][Oo][Nn][Ss].[Mm][Dd]", "**/.github/copilot-instructions.md" ] }, diff --git a/src/guard.mjs b/src/guard.mjs index 5c79a5e..bdd3854 100644 --- a/src/guard.mjs +++ b/src/guard.mjs @@ -161,8 +161,15 @@ export function repairStagedBlocks(repo, blocks, paths) { if (!pending.length) break; unstagePaths(repo, pending); } - return stagedBefore !== null - && stagedBefore.every((path) => unstageTargets.has(path)); + // collateral is what the repair took beyond the blocked paths themselves: + // staged deletions kept as possible rename sources. The caller has to name + // them, or the developer commits a message describing a removal the commit + // no longer carries. + const blocked = new Set(paths); + return { + emptied: stagedBefore !== null && stagedBefore.every((path) => unstageTargets.has(path)), + collateral: [...unstageTargets].filter((path) => !blocked.has(path)), + }; } // resolveIntroduced maps each proposed update to the commits it introduces, diff --git a/tests/cli.test.mjs b/tests/cli.test.mjs index 4d4eabd..192cc6c 100644 --- a/tests/cli.test.mjs +++ b/tests/cli.test.mjs @@ -239,10 +239,13 @@ test('precommit: a zero-similarity blocked rename restores the possible source d execFileSync('git', ['add', '-f', '.playwright-mcp/trace.json'], { cwd: dir }); const out = result('precommit', [], dir); - // Repairing the index leaves nothing staged, so the commit stops - // rather than land empty. + // Repairing the index leaves nothing staged, so the commit stops rather + // than land empty. The deletion it restored was staged work, so the + // report names it instead of claiming nothing else was there. assert.equal(out.status, 10, out.stderr); - assert.match(out.stderr, /nothing else was staged/); + assert.match(out.stderr, /the index is empty now/); + assert.match(out.stderr, /also restored 1 staged deletion\(s\)/); + assert.match(out.stderr, /old\.txt/); const staged = execFileSync('git', ['diff', '--cached', '--name-status'], { cwd: dir, encoding: 'utf8', @@ -735,7 +738,37 @@ test('clean precommit fully unstages a blocked rename', () => { encoding: 'utf8', }).trim(); assert.equal(staged, ''); - assert.match(out.stderr, /nothing else was staged/); + // `git mv` stages a deletion of the source, which the repair restores, + // so the report has to account for it rather than say nothing else was + // staged. + assert.match(out.stderr, /the index is empty now/); + assert.match(out.stderr, /README\.md/); + } finally { rmSync(dir, { recursive: true, force: true }); } +}); + +// The sweep that restores possible rename sources is deliberate, but it takes +// paths the developer staged on purpose. Saying nothing leaves them committing a +// message that describes an untracking the commit does not contain. +test('clean precommit names the staged deletion its repair restored', () => { + const dir = makeRepo('clean'); + try { + writeFileSync(join(dir, 'legacy.txt'), 'tracked\n'); + execFileSync('git', ['add', 'legacy.txt'], { cwd: dir }); + execFileSync('git', ['commit', '--no-verify', '-q', '-m', 'add legacy'], { cwd: dir }); + execFileSync('git', ['rm', '--cached', '-q', 'legacy.txt'], { cwd: dir }); + mkdirSync(join(dir, '.claude'), { recursive: true }); + writeFileSync(join(dir, '.claude/session.json'), '{}'); + execFileSync('git', ['add', '-f', '.claude/session.json'], { cwd: dir }); + + const out = result('precommit', [], dir); + assert.equal(out.status, 10, out.stderr); + assert.match(out.stderr, /legacy\.txt/, 'the repair must name the deletion it restored'); + assert.match(out.stderr, /git rm/, 'and say how to stage the removal again'); + assert.doesNotMatch( + out.stderr, + /nothing else was staged/, + 'something else was staged, and the repair took it', + ); } finally { rmSync(dir, { recursive: true, force: true }); } }); diff --git a/tests/guard.test.mjs b/tests/guard.test.mjs index 16ddf31..d0cf62c 100644 --- a/tests/guard.test.mjs +++ b/tests/guard.test.mjs @@ -141,14 +141,15 @@ test('repairStagedBlocks unstages the blocked paths and reports an emptied index writeFileSync(join(dir, 'keep.txt'), 'keep\n'); git(dir, ['add', '-f', '.claude.json', 'keep.txt']); const partial = repairStagedBlocks(repo, [{ path: '.claude.json' }], ['.claude.json']); - assert.equal(partial, false, 'other work stays staged, so the commit may proceed'); + assert.equal(partial.emptied, false, 'other work stays staged, so the commit may proceed'); + assert.deepEqual(partial.collateral, [], 'nothing beyond the blocked path was taken'); assert.deepEqual(stagedPaths(repo), ['keep.txt']); assert.ok(existsSync(join(dir, '.claude.json')), 'the worktree file survives'); git(dir, ['restore', '--staged', 'keep.txt']); git(dir, ['add', '-f', '.claude.json']); const emptied = repairStagedBlocks(repo, [{ path: '.claude.json' }], ['.claude.json']); - assert.equal(emptied, true, 'the repair removed everything that was staged'); + assert.equal(emptied.emptied, true, 'the repair removed everything that was staged'); assert.deepEqual(stagedPaths(repo), []); } finally { rmSync(dir, { recursive: true, force: true }); diff --git a/tests/rule-contracts.test.mjs b/tests/rule-contracts.test.mjs index d23863e..fee8378 100644 --- a/tests/rule-contracts.test.mjs +++ b/tests/rule-contracts.test.mjs @@ -221,7 +221,7 @@ test('corner-cut labels match case-insensitively without broadening near misses' } }); -test('session and policy paths match case-insensitively; instruction paths do not', () => { +test('session and policy paths match case-insensitively; instruction paths fold case at the root only', () => { const engine = newEngine('strict'); // On APFS and NTFS these are the same file as their lowercase spellings, and // Git records the case the caller typed rather than the case on disk. @@ -231,16 +231,21 @@ test('session and policy paths match case-insensitively; instruction paths do no ['.Codex/sessions/rollout.jsonl', 'codex.session-state'], ['.Cursor/chats/feature.json', 'cursor.session-state'], ['.Aimhooman.json', 'generic.project-policy'], + // Same argument, same filesystems: a root instruction file is the + // guarded one whatever case the caller typed. + ['agents.md', 'generic.agent-instructions'], + ['claude.md', 'generic.agent-instructions'], + ['Gemini.md', 'generic.agent-instructions'], + ['AGENTS.MD', 'generic.agent-instructions'], + ['.github/Copilot-Instructions.md', 'generic.agent-instructions'], ]; for (const [path, ruleId] of cases) { assert.ok(matchFor(engine.checkPaths([path]), ruleId), path); } - // The negative control for the opt-in: generic.agent-instructions stays - // case-sensitive, so folding case is still a per-rule decision rather than a - // global one. Its names lowercase to ordinary documentation filenames, which - // must keep committing freely. - for (const path of ['docs/claude.md', 'docs/gemini.md', 'website/content/blog/agents.md']) { + // The near-miss control: only the root names fold. Deeper in the tree these + // are ordinary documentation filenames and must keep committing freely. + for (const path of ['docs/claude.md', 'docs/gemini.md', 'website/content/blog/agents.md', 'agentsxmd']) { assert.equal(engine.checkPaths([path]).length, 0, path); } });