feat: [ENG-2154] Add OS system/temp files to CONTEXT_TREE_GITIGNORE_P…

[bao-byterover]

Summary

• Problem: .brv/context-tree/ accumulates OS- and editor-generated junk (.DS_Store, Thumbs.db, *.swp, AppleDouble ._*, etc.) when users sync the tree across machines or open editors directly inside it. These leak into commits and pollute the BM25 index.
• Why it matters: A polluted context tree degrades retrieval quality (BM25 indexes junk tokens) and creates noisy diffs when users commit their context tree to their project repo via brv vc.
• What changed: Extended CONTEXT_TREE_GITIGNORE_PATTERNS in src/server/constants.ts with grouped, commented sections for macOS / Windows / Linux / editor swap-backup-temp. Refactored the two variance tests in test/unit/utils/gitignore.test.ts to derive their fixtures from the shared FULL_GITIGNORE constant so they stay in sync with the array.
• What did NOT change (scope boundary): The writer ensureContextTreeGitignore is untouched — its idempotent / negation-respecting / comment-respecting behavior already handles in-place upgrades for existing users. No trash folder patterns (.Trashes, .Trash-*, $RECYCLE.BIN/), no language caches (__pycache__, .cache/), no _archived/.

Type of change

• Bug fix
• New feature
• Refactor (no behavior change)
• Documentation
• Test
• Chore (build, dependencies, CI)

Scope (select all touched areas)

• TUI / REPL
• Agent / Tools
• LLM Providers
• Server / Daemon
• Shared (constants, types, transport events)
• CLI Commands (oclif)
• Hub / Connectors
• Cloud Sync
• CI/CD / Infra

Linked issues

• Closes ENG-2154
• Related #

Root cause (bug fixes only, otherwise write N/A)

• Root cause: N/A
• Why this was not caught earlier: N/A

Test plan

• Coverage added:
  • Unit test
  • Integration test
  • Manual verification only
• Test file(s):
  • test/unit/server/constants.test.ts — new describe('OS-generated junk files') block with 5 assertions
  • test/unit/utils/gitignore.test.ts — refactored two fixtures to derive from FULL_GITIGNORE so future additions to the constant do not silently drop test coverage
• Key scenario(s) covered:
  • macOS patterns present (.DS_Store, ._*)
  • Windows patterns present (Thumbs.db, ehthumbs.db, Desktop.ini)
  • Linux patterns present (.directory, .fuse_hidden*, .nfs*)
  • Editor swap / backup / temp patterns present (*.swp, *.swo, *~, .#*, *.bak, *.tmp)
  • Negative assertion: trash folders (.Trashes, .Trash-*, $RECYCLE.BIN/) are deliberately NOT included (out of scope per ENG-2154)
  • Existing ensureContextTreeGitignore behavior tests (idempotency, negation !pattern, comment # pattern, whitespace handling, /pattern variant suppression) remain green against the enlarged pattern set

User-visible changes

• Newly-created .brv/context-tree/.gitignore files include 14 additional ignore patterns covering common OS- and editor-generated junk.
• Existing .brv/context-tree/.gitignore files will be upgraded in place on next daemon write — the writer appends only missing patterns, preserves user edits (including !negations and # comments), and never rewrites the file when all patterns are already covered.
• No CLI flag, config key, or command-output changes.

Evidence

• Failing test/log before + passing after

# Before constant update — new OS/junk assertions fail
$ npx mocha --forbid-only "test/unit/server/constants.test.ts"
  CONTEXT_TREE_GITIGNORE_PATTERNS
    ✓ should include derived artifact patterns
    OS-generated junk files
      1) should exclude macOS junk (Finder metadata + AppleDouble forks)
      2) should exclude Windows junk (thumbnail cache + folder config)
      3) should exclude Linux junk (KDE metadata + FUSE/NFS hidden files)
      4) should exclude editor swap / backup / temp files
      ✓ should deliberately NOT include trash folders (out of scope per ENG-2154)

# After constant update — all green
$ npx mocha --forbid-only "test/unit/server/constants.test.ts"
  6 passing

$ npx mocha --forbid-only "test/unit/utils/gitignore.test.ts"
  (all behavioral tests pass against the enlarged pattern set)

[github-actions]

Claude finished @bao-byterover's task in 2m 3s —— View job

---

PR Review: feat/ENG-2154 — Add OS system/temp files to CONTEXT_TREE_GITIGNORE_PATTERNS

Todo List

• Gather context and read changed files
• Review src/server/constants.ts
• Review test/unit/server/constants.test.ts
• Review test/unit/utils/gitignore.test.ts
• Review src/server/utils/gitignore.ts (unchanged writer, relevant for impact)
• Post inline comments
• Post final review summary

---

Summary

Overall Score: 4.5 / 5

This is a clean, well-scoped PR with excellent discipline: the scope boundary is clearly stated in the PR description and enforced by a negative test assertion. The TDD approach is solid — failing tests before the constant update, passing after. The fixture refactoring in gitignore.test.ts is a genuine improvement that keeps future constant additions covered automatically.

---

What's Good

• Scope control: Deliberately excluding trash folders (.Trashes, $RECYCLE.BIN/) and documenting the reason is excellent practice. The negative assertion in constants.test.ts:48–52 codifies this as an enforced invariant.
• Fixture refactoring: Replacing two hardcoded multi-line strings in gitignore.test.ts with FULL_GITIGNORE.replace(...) is strictly better — future additions to CONTEXT_TREE_GITIGNORE_PATTERNS will no longer silently drop those two variance tests.
• Pattern selection: The 14 chosen patterns are all genuinely unambiguous OS/editor junk. .* prefixed patterns, .fuse_hidden*, .nfs* are all standard gitignore conventions.
• Backward compatibility: The idempotent ensureContextTreeGitignore writer appends-only, so existing context trees get a safe in-place upgrade.

---

Issues Found

1. CONTEXT_TREE_GITIGNORE_HEADER is stale after this change

File: src/server/constants.ts:156

export const CONTEXT_TREE_GITIGNORE_HEADER = '# Derived artifacts — do not track'

The header is used verbatim as the first line of every generated .gitignore. With 14 new OS/editor junk entries now in the array, "Derived artifacts" no longer describes the full contents. Users who open .brv/context-tree/.gitignore in an editor will see a header that doesn't match. Suggest updating to something like # Context tree — auto-managed, do not track or # Derived artifacts and OS junk — do not track.

(Inline comment posted on the changed JSDoc line above the constant.)

2. Pre-existing writer logic amplified by larger pattern set

File: src/server/utils/gitignore.ts:74 (unchanged, but worth flagging)

if (existingLines.some((el) => el.includes(pattern))) continue

String.includes() is a substring check. With new patterns:

• .directory would be skipped if any existing line contains .directory as a substring (e.g. .directories/)
• *.tmp would be skipped if a line like _*.tmp exists (which doesn't fully cover *.tmp)

This is pre-existing behavior and not introduced here, but expanding the pattern set increases the surface. Not a blocker — the writer has best-effort semantics and the risk is low in practice.

3. Typo in test name (pre-existing line, not in diff)

File: test/unit/utils/gitignore.test.ts:227

it('should upgrade from pre-ENG-2014 version — append only new patterns', ...
//                         ^^^^^^^^
//                         Should be ENG-2154 (the current ticket)

The line wasn't changed in this PR so an inline comment couldn't be posted, but it's worth fixing.

---

Minor Observations

• *.bak and *.tmp are intentionally broad, which is correct for a context-tree where no legitimate knowledge document should carry these extensions.
• The ._* AppleDouble pattern could theoretically match intentional user files, but in a .brv/context-tree/ context this is an acceptable trade-off.
• No missing patterns that fall within the stated scope (macOS / Windows / Linux / editor swap-backup-temp).

---

Verdict: Ready to merge with the header constant update being the only thing worth addressing before ship (minor but user-visible). The typo fix is cosmetic.