Exclude untracked dotfiles (e.g. .claude/) from generated patches - #36
Exclude untracked dotfiles (e.g. .claude/) from generated patches#36juanmaguitar wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the app’s generated patch logic to avoid including untracked dotfiles/dot-directories (e.g. agent tool config like .claude/) while keeping already-tracked dotfiles (e.g. .gitignore) included. It also refactors patch generation into a standalone, unit-testable module and adds coverage for the new exclusion behavior.
Changes:
- Extracted patch generation into
src/patch.jsand updated the Electron main process to call it. - Added
isDotPath()filtering to skip untracked dotfile/dotdir paths during staging and when building the diff list. - Added
test/patch.test.cjsto validate dotpath exclusion and tracked-dotfile modifications.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/patch.js |
New module containing the extracted patch generation logic plus dotpath filtering. |
src/main.js |
Switches patch generation to use the new src/patch.js module. |
test/patch.test.cjs |
Adds unit tests for dotpath exclusion and tracked dotfile modifications. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
test/patch.test.cjs:78
- This test creates a temporary repo directory but doesn't clean it up afterwards. Wrapping the body in a try/finally and removing the directory helps avoid accumulating temp dirs over many test runs.
test('createMinimalPatchForDir still includes modifications to already-tracked dotfiles', async () => {
const dir = await makeRepoWithTrunkCommit();
fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\n');
await git.add({ fs, dir, filepath: '.gitignore' });
await git.commit({
fs,
dir,
message: 'add .gitignore',
author: { name: 'Test', email: 'test@example.com' },
});
await git.writeRef({
fs,
dir,
ref: 'refs/remotes/origin/trunk',
value: await git.resolveRef({ fs, dir, ref: 'HEAD' }),
force: true,
});
fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\nbuild/\n');
const patch = await createMinimalPatchForDir(dir);
assert.match(patch, /\.gitignore/);
assert.match(patch, /build\//);
});
test/patch.test.cjs:106
- This test creates a temporary git repo but doesn't remove it after the assertions run. Cleaning up in a finally block keeps the test suite from leaving behind many patch-test-* directories in the temp folder.
test('createMinimalPatchForDir includes deletions of tracked files', async () => {
const dir = await makeRepoWithTrunkCommit();
// Create and commit a tracked file, and point origin/trunk at it.
fs.writeFileSync(path.join(dir, 'delete-me.txt'), 'bye\n');
await git.add({ fs, dir, filepath: 'delete-me.txt' });
await git.commit({
fs,
dir,
message: 'add file to delete',
author: { name: 'Test', email: 'test@example.com' },
});
await git.writeRef({
fs,
dir,
ref: 'refs/remotes/origin/trunk',
value: await git.resolveRef({ fs, dir, ref: 'HEAD' }),
force: true,
});
fs.rmSync(path.join(dir, 'delete-me.txt'));
const patch = await createMinimalPatchForDir(dir);
assert.match(patch, /delete-me\.txt/);
assert.match(patch, /-bye/);
});
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
test/patch.test.cjs:60
- This test creates a temporary repo directory but never removes it, which can leak temp files and cause flaky/slow CI over time. Consider registering cleanup via node:test's
t.after()so the temp dir is removed even if assertions fail.
test('createMinimalPatchForDir still includes modifications to already-tracked dotfiles', async () => {
const dir = await makeRepoWithTrunkCommit();
fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\n');
test/patch.test.cjs:86
- This test creates a temporary repo directory but never removes it, which can leak temp files and cause flaky/slow CI over time. Register cleanup via node:test's
t.after()so the temp dir is removed even if the test throws or an assertion fails.
test('createMinimalPatchForDir includes deletions of tracked files', async () => {
const dir = await makeRepoWithTrunkCommit();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
test/patch.test.cjs:65
- This test creates a temp repo directory but never removes it. That can leak temp folders across test runs and eventually cause failures on CI machines with limited disk space.
test('createMinimalPatchForDir still includes modifications to already-tracked dotfiles', async () => {
const dir = await makeRepoWithTrunkCommit();
fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\n');
await git.add({ fs, dir, filepath: '.gitignore' });
await git.commit({
test/patch.test.cjs:89
- This test creates a temp repo directory but doesn't clean it up. Using
t.after()is a low-impact way to ensure cleanup even if an assertion fails.
test('createMinimalPatchForDir includes deletions of tracked files', async () => {
const dir = await makeRepoWithTrunkCommit();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
test/patch.test.cjs:63
- This test creates a temp repo directory but never removes it, which can leak many
patch-test-*directories across test runs. Add cleanup (e.g.t.after(...)or atry/finally) like the earlier test does.
test('createMinimalPatchForDir still includes modifications to already-tracked dotfiles', async () => {
const dir = await makeRepoWithTrunkCommit();
fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\n');
test/patch.test.cjs:89
- This test creates a temp repo directory but never removes it, which can leak many
patch-test-*directories across test runs. Add cleanup (e.g.t.after(...)or atry/finally) like the first test does.
test('createMinimalPatchForDir includes deletions of tracked files', async () => {
const dir = await makeRepoWithTrunkCommit();
Untracked dotfiles and dotdirs — .claude/, .cursor/ and whatever the next tool leaves behind — were being staged and diffed into the patch a contributor attaches to Trac. The fix is a Git exclude rather than a filter in the app: when a site is created, .git/info/exclude gets a marked block with a `.*` rule. isomorphic-git honours it when walking the status matrix, so excluded entries never reach the diff at all. Being ordinary exclude rules, a contributor can narrow them per site; the block is rewritten only when its marker line is missing, so edits below it survive. Existing sites are covered too: entries an older app version already staged are unstaged when they turn out to be ignored. Patch generation moves out of main.js into src/patch.js, which is what makes any of it testable — main.js cannot be required outside an Electron process. Also fixes a deletion being dropped from the patch: a tracked file removed from the working tree has no workdir buffer, and the old code fell back to the HEAD content, making both sides equal so the diff came out empty. It now compares against empty text, and a test covers it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
21c99f8 to
0cf3ce5
Compare
Summary
Fixes #19. Agent-tool config directories like
.claude/were showing up in patches generated by the app, because untracked files are staged and diffed regardless of what they are.Per review feedback from @bacoords, exclusion is now gitignore-based rather than hardcoded: the app writes a default rule (
.*, i.e. untracked dotfiles/dotdirs) into each site's.git/info/exclude, which isomorphic-git honors when building the status matrix.A global gitignore wouldn't work here — isomorphic-git never reads
core.excludesFilefrom~/.gitconfig, and the app's target audience (first-time contributors) has no Git installed or configured at all. The per-repo exclude file gets the same effect with two advantages: advanced users can edit the rules per site (the app never rewrites the file once its marker line is present), and the file itself never shows up in generated patches.Rebased onto current trunk
This branch was cut before #94, #111 and #121 landed, and
createMinimalPatchForDirmoved on underneath it.src/patch.jsis rebuilt on trunk's version of that function rather than the one the branch forked from, so it keeps everything the branch never saw:ensureAutocrlf, the CRLF→LF normalisation on both sides of the diff, and the comment explaining why the diff base is the local HEAD rather than the remote trunk ref.The deletion fix below had to be re-applied by hand on top of that — taking trunk's line verbatim reintroduced the bug, and
test/patch.test.cjscaught it. Worth knowing if this branch is ever rebased again.The twelve original commits (eight of them
Potential fix for pull request finding) are squashed into one. The repo squash-merges anyway.Changes
src/main.jsintosrc/patch.js(no Electron dependency, so it's unit-testable).ensureDefaultExcludes(): writes the default exclusion rules to.git/info/excludeat clone time and before each patch generation. Skipped once the marker line exists, so user edits survive..gitignore) are unaffected — ignore rules only apply to untracked files.test/patch.test.cjscovering exclusion, tracked-dotfile edits, deletions, legacy staged dotfiles, and user overrides of the exclude rules.Test plan
Automated
npm testpasses (164/164, including 17 new tests intest/patch.test.cjs)npm run lintcleanFull end-to-end in the app
wordpress-developclone.npm start# wp-contributor-toolkit defaultsmarker line followed by.*.README.mdchange; neither.claude/nor.cursor/appears.<site>/.git/info/excludeand replace.*with.claude/, keeping the marker line. Regenerate the patch. Expected:.cursor/rules.mdnow appears,.claude/still doesn't — confirming the rules are user-controllable rather than hardcoded..claude/was already staged into the index): generate a patch and confirm.claude/is gone from it. To simulate on a fresh site, stage it manually first:Note that steps 3-7 don't require the "Install npm dependencies" step to have succeeded — patch generation only needs the clone. That step is currently broken on
trunkfor an unrelated reason (Electron 32 bundles Node v20.18.1, whilewordpress-develop's.npmrcsetsengine-strict = trueand its transitive dep@eslint/compatrequires Node^20.19.0 || ^22.13.0 || >=24). That's being tracked separately and is out of scope here.