Fix test-suite portability (paths with spaces, git security defaults) and person-link bugs - #427
Merged
Merged
Conversation
…bugs
Four fixes from power-user fork feedback (thanks Michelle):
1. NODE_OPTIONS preload paths are now double-quoted in every test that
builds `--require=<path>`. NODE_OPTIONS is whitespace-parsed, so an
unquoted vault/repo path containing a space (e.g. iCloud's
"Mobile Documents") silently truncated the path and failed ~60 tests.
Node accepts double quotes inside NODE_OPTIONS (verified empirically
on Node 22).
2. .distignore (and its .gitattributes mirror) now excludes five
personal/machine-local paths real forks track: .codex/,
.obsidianignore, _import/, bun.lock, memory/ — so a distribution
build from a fork never sweeps them up as shippable content.
3. vault-autocommit.test.cjs's git helper passes
-c safe.bareRepository=all so its own throwaway bare remote works
under the increasingly common git security default
safe.bareRepository=explicit. Scoped to the helper only; never
touches real git config.
4. .scripts/auto-link-people.cjs (both reports verified by running the
script against a fixture before changing anything):
- wikilink targets are now the bare page name ([[Jane_Smith|Jane
Smith]]) instead of the full vault-relative file path; bare links
also survive a page moving between Internal/ and External/.
- every eligible mention of a person is linked, not just the first
occurrence per file.
Tests updated to encode the new behavior; dead link-consumption
code removed; CLAUDE.md + System Guide prose aligned.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What this pull request touches
Gates that will judge this change
Based on 15 changed files. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
Four fixes reported by power user Michelle, who found them by reconciling her fork against upstream and actually running everything — full credit to her for the careful, run-it-don't-read-it verification. All four were confirmed by reproducing them here before changing any code.
1. Tests broke for anyone whose Dex folder path contains a space
About 60 tests failed for Michelle because her vault lives under iCloud's "Mobile Documents" folder — a path with a space in it. The test files passed a helper-file location to Node through a setting that splits on spaces, so the path got cut off mid-way and the tests couldn't find their own helper.
Fix: the path is now wrapped in quotes everywhere it's passed (10 places across 7 test files). Verified the quoting form Node actually accepts by testing it directly, not assuming.
Proof: ran the affected suites from a copy of the repo placed at a folder path containing a space:
connections-contractsuite before: 2 of 4 tests fail with "Cannot find module '/tmp/dex'" (the path cut off at the space). After: 4/4 pass.adapter-runnersuite with a temp folder containing a space before: 1 fail. After: 7/7 pass.2. Distribution builds could sweep up personal files from forks
Five kinds of personal or machine-local files that real forks track (
.codex/,.obsidianignore,_import/,bun.lock,memory/— including Claude Code's own memory folder) were missing from the distribution exclusion list, so a release built from such a fork would have shipped them. Added to.distignoreand its.gitattributesmirror. All 35 distribution/quarantine tests still pass.3. One test couldn't run under a common git security setting
A test creates its own throwaway "remote" repository to prove the autocommit hook never pushes. Under the increasingly common git security default
safe.bareRepository=explicit, the test's own helper couldn't operate on that throwaway repository. Reproduced exactly (test 5 fails with "cannot use bare repository"), then fixed with an override scoped to that one test helper — nobody's real git configuration is touched. Suite passes under the security default now (7/7).4. Person auto-linking: two real bugs, both verified by running it
Michelle caught these by running the script against a test file, so we did the same before touching code. Both reproduced on main:
[[05-Areas/People/External/Jane_Smith|Jane Smith]]) instead of the bare page name ([[Jane_Smith|Jane Smith]]). Bare names read cleaner and keep working when a person's page moves between the Internal and External folders.Note for review: the old behavior was encoded in the existing tests (it was deliberate at the time), so this is a behavior change, not just a bug fix — the tests now encode the new behavior, and the one prose line in CLAUDE.md and the System Guide describing path-based links was updated to match. The meeting-notes participants line (a separate code path) still uses path links and was left untouched.
Test results
DEX_PYTHONset; the 11 parity tests that need it fail identically on main without it — pre-existing, environment-only)Proposed changelog entry (not committed — for the release that includes this)
🤖 Generated with Claude Code