feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(server): clean worktree build artifacts when threads settle - #8771

Open
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts
Open

feat(server): clean worktree build artifacts when threads settle#8771
IAmJSD wants to merge 6 commits into
pingdotgg:mainfrom
Infrawrench:clean-settled-worktree-artifacts

Conversation

@IAmJSD

@IAmJSDIAmJSD commented Aug 30, 2026

Copy link
Copy Markdown

Note

This PR was written by Claude Code (Claude Fable 5) under human direction — the code, the tests, and this description included.

What Changed

  • Added an opt-in Clean settled worktrees server setting (default off) in Settings → General, next to the existing worktree options, with search/reset/modified-count support.
  • Added a ThreadSettleCleanupReactor (modeled on ThreadDeletionReactor) that reacts to thread.settled domain events. When the settled thread runs in a linked worktree, it deletes regenerable build artifacts: node_modules, Cargo target (only when next to a Cargo.toml), .next, .nuxt, .turbo, .svelte-kit.
  • Safety guards, in order: setting must be on; thread must have a live worktreePath not shared with another live thread; the path must be a linked worktree (.git pointer file — a primary checkout is never touched). The scan never enters .git or matched artifacts, never follows symlinks out of the worktree, and removal is best-effort per directory. Tracked files and uncommitted work are never at risk.
  • Tests: real-tempdir coverage for the artifact scanner (depth, Cargo gating, symlink escape, .git-pointer detection), unit coverage for the shared-worktree guard, and the OrchestrationReactor start-order test updated.

Scope note: cleanup fires on explicit thread.settled events; client-derived auto-settle (inactivity / merged PR) emits no domain event and is deliberately out of scope.

Why

Settled worktree threads keep their install and build caches on disk forever — a handful of parked JS/Rust threads easily holds gigabytes. This reclaims that disk the moment a thread is parked while keeping the worktree itself intact (branch, tracked files, uncommitted changes), so an unsettled thread simply resumes and the setup script regenerates caches on the next turn.

Complementary to worktree pruning proposals like #4742: those remove whole worktrees under retention policies; this keeps the worktree and strips only regenerable artifacts, triggered by settling.

UI Changes

One standard settings row (title, description, Switch) in Settings → General. No screenshots included; happy to add them on request.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Written by Claude Code with Claude Fable 5 (claude-fable-5).

🤖 Generated with Claude Code


Note

Medium Risk
Opt-in filesystem deletes on worktrees, but gated by linked-worktree verification, git ignore/tracked-file checks, and shared-thread guards; primary checkouts are never touched.

Overview
Adds an opt-in Clean settled worktrees setting (default off) and a ThreadSettleCleanupReactor that runs when a thread.settled domain event is emitted.

When enabled, the reactor removes regenerable artifacts (node_modules, framework caches, Cargo target next to Cargo.toml) only from linked git worktrees, after checks that the thread is still settled, not deleted, not sharing the path with another live thread, and that git treats each directory as ignored with no tracked files. Scanning is depth-bounded, skips .git and symlinks that escape the tree, and deletion is best-effort per directory.

New worktreeArtifacts helpers implement discovery, linked-worktree detection, and guarded removal. The reactor is started from OrchestrationReactor and registered in the server reactor layer; Settings → General gets a toggle plus search/reset/dirty tracking.

Reviewed by Cursor Bugbot for commit 12d9550. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add ThreadSettleCleanupReactor to clean worktree build artifacts on thread settle

  • Introduces a new reactor that subscribes to thread.settled events and removes regenerable build artifact directories (e.g. node_modules, .next, .turbo) from linked git worktrees
  • Adds utilities in worktreeArtifacts.ts: isLinkedWorktreePath detects linked worktrees, findWorktreeArtifactDirectories does a bounded-depth walk over an allowlist, and removeWorktreeArtifacts verifies candidates are git-ignored with no tracked files before recursive deletion
  • Adds cleanWorktreeArtifactsOnSettle boolean to ServerSettings (default false) and ServerSettingsPatch, with a toggle in the General settings UI and settings search entry
  • Wires the reactor into OrchestrationReactor.start() and the live server layer composition
  • Guards skip cleanup when the setting is off, the thread is deleted, another live thread shares the worktree, or the path is not a linked worktree
  • Risk: removeWorktreeArtifacts recursively force-deletes directories; a false positive in the git-ignore/track verification would delete uncommitted files. Reviewers should check removeWorktreeArtifacts in worktreeArtifacts.ts and the shared-worktree guard isWorktreeSharedWithAnotherThread in ThreadSettleCleanupReactor.ts

Macroscope summarized 12d9550.

Settled worktree threads keep their node_modules, Cargo target, and
framework cache directories around indefinitely, so parked threads eat
disk. A new ThreadSettleCleanupReactor reacts to thread.settled events
and deletes regenerable build artifacts from the thread's linked
worktree, behind a new opt-in "Clean settled worktrees" server setting.
Cleanup only touches linked worktrees (never a primary checkout), skips
worktrees shared with another live thread, never follows symlinks, and
never removes tracked files.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 499885c4-ba8c-4a40-82a2-30197a2a5503

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actionsgithub-actionsBot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions: the new ThreadSettleCleanupReactor service is introduced with the legacy Services/ + Layers/ split, a standalone ...Shape interface, and a ...Live layer export. The repo's canonical shape for new services (see orchestration/ThreadBackgroundLiveness.ts, relay/AgentAwarenessRelay.ts, workspace/WorkspacePaths.ts) is a single module exporting the tag with an inline interface, make, and layer. Details inline.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/Layers/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/Services/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Comment threadapps/server/src/git/worktreeArtifacts.ts Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1003ff8. Configure here.

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
@macroscopeapp

macroscopeappBot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a new event-driven worktree cleanup capability with recursive filesystem deletion, production reactor wiring, and a user-facing product setting. Despite being opt-in and defaulting off, the feature’s scope and destructive side effect require human review.

You can add or adjust custom eligibility rules. Learn more.

Collapse the new reactor into a single canonical module (tag with inline
interface, make, layer) per current Effect service conventions; re-check
the projected settled state before cleaning so a stale queued settle
event cannot clean under a thread that has since unsettled; and require
Cargo.toml to be a regular file before treating a sibling target
directory as a build artifact.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed all three review findings in 35c7822 (fix written by Claude Code, as is this comment):

  • Effect service conventions — collapsed the reactor into a single canonical module at apps/server/src/orchestration/ThreadSettleCleanupReactor.ts (tag with inline interface, make, layer); removed the Services/ + Layers/ split and the standalone Shape type, updated all importers to ThreadSettleCleanupReactor["Service"] member types.
  • Stale settle race — the worker now re-checks settledOverride === "settled" on the freshly loaded projection row before cleaning. Events are projected inside the commit transaction before being published, so a thread unsettled after the settle event was queued no longer reads as settled by dequeue time.
  • Cargo.toml directory false positive — the scanner now stats the manifest and requires a regular file before treating a sibling target directory as a Cargo build artifact, with a regression test.

🤖 Generated with Claude Code

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
A primary checkout cloned with --separate-git-dir also keeps a .git
pointer file, so the entry type alone could let settle cleanup touch a
primary checkout. Resolve the pointer's gitdir target and require its
commondir file, which only a linked worktree's private git dir carries.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Fixed the separate-git-dir finding in 5a0e290 (written by Claude Code, as is this comment): isLinkedWorktreePath now resolves the .git pointer's gitdir: target and requires its commondir file — present only in a linked worktree's private git dir, absent in a --separate-git-dir clone's full git dir (verified against real git worktree add / git clone --separate-git-dir layouts). Malformed pointers read as "not a worktree", so cleanup never guesses. Tests updated with faithful on-disk fixtures for all three cases.

The earlier Cursor finding ("cleanup runs after thread unsettles") was already addressed in 35c7822 via the settledOverride === "settled" re-check.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed for Effect service conventions. The service is now a single canonical module (apps/server/src/orchestration/ThreadSettleCleanupReactor.ts) with the interface inline in Context.Service, Service["Service"] references, make, and layer — the layout findings from the previous run are resolved. One remaining item: the reactor's new backend behavior has no focused test.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Adds focused reactor tests driving a real thread.settled event through
make with stubbed engine/repository/settings layers and real filesystem
fixtures: cleanup fires only when the setting is enabled and every
guard passes; disabled setting, unsettled or deleted threads, shared
worktrees, and primary checkouts are all left untouched. Aligns the
reactor with the sibling deletion reactor's seen-sequence watermark and
drainThrough so the tests wait on receipts instead of sleeps.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed the test-coverage finding in 6144972 (written by Claude Code, as is this comment): added focused reactor tests that drive a real thread.settled event through make with stubbed engine/repository/settings layers and real on-disk fixtures, asserting cleanup fires only when the setting is enabled and every guard passes — and is skipped for a disabled setting, an unsettled or deleted thread, a shared worktree, and a primary checkout. The reactor now also mirrors ThreadDeletionReactor's seen-sequence watermark and exposes drainThrough instead of the previously unused drain, so the tests wait on receipts rather than timing.

🤖 Generated with Claude Code

@macroscopeappmacroscopeappBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect Service Conventions: one finding — namespace-erasing aliased imports in the new reactor test. Everything else in the new ThreadSettleCleanupReactor module (canonical single-file layout, inline Context.Service interface, make/layer, environment-acquired dependencies, Foo["Service"] references) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.test.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts Outdated
Comment threadapps/server/src/orchestration/ThreadSettleCleanupReactor.ts
Hardens settle cleanup against the two review criticals: artifact
directories are now deleted only after git confirms they are ignored
and hold no tracked files (anything unverified is skipped and logged),
and isLinkedWorktreePath additionally requires the private git dir's
gitdir back-reference to resolve to this worktree's own .git pointer,
so a pointer borrowed from another worktree vouches for nothing.
Artifact and reactor tests now run against real git repositories and
worktrees, including an impostor-pointer case and unverified-skip
cases.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Addressed both criticals in c8bf5e8 (written by Claude Code, as is this comment):

  • Name-based deletion could hit tracked/uncommitted workremoveWorktreeArtifacts now deletes a directory only after git verifies it: git check-ignore must report it ignored and git ls-files must show no tracked files inside. Anything unverified (including any failed git invocation) is skipped untouched and logged. Covered by new tests against real repos: verified artifacts removed, non-ignored node_modules skipped, non-git directories skipped entirely.
  • .git pointer borrowed from another worktreeisLinkedWorktreePath now also resolves the private git dir's gitdir back-reference and requires it to canonically match this worktree's own .git pointer, so only the directory git actually registered passes. Tested with a real git worktree add checkout plus an impostor directory carrying a copied pointer.

On the Medium watermark finding (event committed between the latestSequence read and stream subscription): this mirrors the exact watermark pattern ThreadDeletionReactor ships on main, and the window is not reachable in practice — reactors start during the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription is live. Cleanup is also deliberately best-effort: a hypothetically missed event costs one uncleaned worktree until its next settle, not correctness. Happy to revisit if the shared pattern gets a common fix, but I'd rather not fork this reactor's subscription semantics from its sibling inside this PR.

🤖 Generated with Claude Code

Drops the latestSequence head-noting from the settle cleanup reactor:
seenSequence now only advances for events the subscription actually
handed to the worker, so drainThrough can never report coverage of an
event committed between the head read and subscription start. This
reactor's drainThrough has no production callers to hang on
pre-subscription sequences; reactors also start behind the command
readiness gate, so no settle can commit before the subscription is
live.
Written by Claude Code.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@IAmJSD

Copy link
Copy Markdown
Author

Follow-up on the Medium watermark finding — rather than leaving it at the written dismissal, 12d9550 (written by Claude Code, as is this comment) removes the latestSequence head-noting entirely: seenSequence now advances only for events the subscription actually handed to the worker, so drainThrough can never claim coverage of an event committed between a head read and subscription start — the integrity gap the finding described no longer exists. Unlike ThreadDeletionReactor, this reactor's drainThrough has no production callers that could hang on pre-subscription sequences (it's a test receipt), so dropping the head-noting is safe here. The remaining "event committed before the subscription is live" window is gated by startup ordering: reactors start inside the startup phase behind the command-readiness gate, so no thread.settled can commit before the subscription exists.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@IAmJSD