Skip to content

fix: make dependencies available in isolated worktrees - #321

Merged
chaitanyagiri merged 1 commit into
HarnessMD:mainfrom
aaroncoville:fix/isolated-worktree-dependencies
Sep 6, 2026
Merged

chaitanyagiri merged 1 commit into
HarnessMD:mainfrom
aaroncoville:fix/isolated-worktree-dependencies

Conversation

@aaroncoville

Copy link
Copy Markdown

What & why

Spawning an agent into an isolated worktree gives it a checkout with no node_modules. Typecheck then fails on missing type definitions and tests and build fail on missing packages.

This links the base checkout's dependencies into the worktree right after the worktree is created successfully, and removes that link again before the retention and garbage-collection checks so an untracked entry cannot make a worktree look permanently dirty. An existing entry is left alone, and a link failure is logged without blocking the spawn, so the worst case is the behaviour we already have today.

Two notes for the reviewer. Cleanup only removes an entry that is a symlink and resolves to the base checkout's node_modules a real directory, or a link pointing anywhere else, is left alone, and there are tests pinning both refusals. And because the worktree shares the base's node_modules, an npm install run inside a worktree writes through the link into the base checkout; that is inherent to sharing rather than copying.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

Before

CleanShot 2026-08-25 at 17 39 10@2x

A fresh isolated worktree, running the project's own typecheck:

$ npm run typecheck
error TS2688: Cannot find type definition file for 'electron-vite/node'.
error TS2688: Cannot find type definition file for 'node'.

With the dependency-link step removed, three of the regression tests fail:

not ok 553 - links the base node_modules into an isolated worktree
not ok 556 - does not follow the dependency symlink when removing a worktree
not ok 558 - reports a failed link without throwing
# tests 559
# pass 556
# fail 3

After

CleanShot 2026-08-25 at 17 39 54@2x

The same command in the same worktree, with the dependencies linked:

$ npm run typecheck
errors: 0

Full suite with the change applied:

# tests 564
# pass 564
# fail 0
$ npm run build
build_exit=0

How I tested it

  • OS: macOS (darwin 25.6.0), Munder Difflin 0.4.5
  • Steps:
    1. Created a detached worktree from main with no node_modules and ran npm run typecheck — reproduced the two TS2688 errors above.
    2. Linked the base checkout's node_modules and re-ran the identical command in the identical worktree — 0 errors.
    3. npm run test:focused on a clean branch cut from main: 564/564 (baseline on main is 552; this adds 12).
    4. Removed the symlink operation and re-ran the suite to confirm the tests actually bite — 3 fail, listed above; restored and back to 559/559.
    5. npm run typecheck — 0 errors. npm run build — succeeds.

The deletion guards are covered by name: removing the symlink type check makes does not remove a real worktree node_modules directory fail, and removing the resolved-target comparison makes does not remove a worktree node_modules link to another directory fail. Teardown safety was the other case worth being careful about: git worktree remove --force must not follow the link and delete the base checkout's node_modules. Test 556 covers that, and it asserts the symlink exists before the removal runs.

Checklist

  • Before and after evidence is attached above, under both headings.

An isolated worktree does not include the base checkout's dependencies, so project validation can fail before it checks an agent's change. Link the dependencies after creating the worktree, while preserving existing entries and allowing the spawn to continue if linking fails.

Before deciding whether a worker worktree is dirty, remove only a dependency link that resolves to the base checkout's node_modules. This keeps unignored links from preventing automatic cleanup while preserving real directories and foreign links. Use Windows junctions so directory links work without elevated privileges.

The regression coverage verifies link creation, skip and failure paths, cleanup without a .gitignore, Windows link selection, and that git worktree removal leaves the base dependencies intact.
mrlfarano added a commit to mrlfarano/munder-difflin that referenced this pull request Aug 31, 2026
@chaitanyagiri

Copy link
Copy Markdown
Collaborator

Reviewed by an agent on the Munder Difflin hive floor, posted from @chaitanyagiri.
First person statements below are the agent's, including the scope limits.
Replies here are read. Questions in this comment are real questions.
Base: origin/main at 956bfb4.


The two unlink call sites are ordered correctly and the difference between them is deliberate in
the right direction: the gc path refuses to reclaim when the unlink fails, and the finalize path
logs and carries on, where a surviving symlink makes git status dirty and the worktree is
preserved rather than discarded. Both end up fail-safe. Only the gc one says so out loud.

The thing I would want in the PR body is the isolation caveat, because right now it is not
written anywhere.
A worktree exists so an agent's work does not touch the user's checkout. A
symlinked node_modules is a hole straight through that: an agent that runs npm install in its
isolated worktree writes into the base checkout's real node_modules, and an npm ci there
rebuilds the user's tree from scratch. That may well be the right trade, the alternative is
copying hundreds of megabytes per agent, but "isolated" now means "isolated except dependencies"
and an agent that believes it is sandboxed will install things.

Second, the link makes the worktree dirty for its whole life, not just at teardown. Your own
test says so:

assert.notEqual(git(wtPath, 'status', '--porcelain'), '', 'the unignored link makes the worktree dirty');

The two teardown paths are handled. What about the agent itself? An agent that runs git status
sees an untracked node_modules, and one that runs git add -A commits a symlink pointing at the
user's home directory onto its branch. Writing node_modules into the worktree's
.git/info/exclude at link time is one line, is per worktree so it never touches the user's
.gitignore, and removes the whole class. Worth considering.

On the last three tests: they regex the text of src/main/index.ts rather than run anything, so
they pin that the call sites are written in the right order and would still pass if
linkWorktreeDeps were a no op. That is a reasonable way to pin ordering when the real path needs
a whole spawn, and it is worth a comment saying that is what they are, so nobody reads them as
behavioural coverage.

does not remove a worktree node_modules link to another directory is the test I would have
forgotten to write. Comparing realpath on both sides rather than trusting the link text is the
right check.

@chaitanyagiri
chaitanyagiri merged commit 12096fd into HarnessMD:main Sep 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants