Skip to content

refactor(runtime): remove unused text-search spawn hook - #27418

Merged
nwparker merged 1 commit into
mainfrom
nwparker/drop-dead-text-search-spawn-hook
Oct 11, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/drop-dead-text-search-spawn-hook

Conversation

@nwparker

@nwparker nwparker commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 0 0 0 0
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3

ELI5

Removes a leftover hook that nothing uses anymore. Search behaves exactly the same.

What Changed

  • Remove the onTextSearchSpawn option from the local filesystem provider and its passthrough in requireRuntimeFileProvider.
  • The local provider now passes a no-op spawn hook to ripgrep.

Why

#27396 deleted the write-only registry that was the hook's only producer, so nothing sets the option anymore. Its doc comment ("Registers a running text search for cancellation") implied a cancellation path that no longer exists. Request abort signals still cancel runtime searches. The renderer IPC search keeps its own by-id registry through runBundledRipgrepTextSearch's onSpawn, which this PR doesn't touch.

Linked Issue

Follow-up to #27396.

Visual Proof

N/A — dead-code removal with no UI or behavior change.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

No new tests: only unused code is removed. pnpm tc:node, the provider/search/ripgrep suites (50 tests) and the changed-code quality gate pass.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Local, Windows, SSH and folder workspaces aren't affected: SSH providers never received this option, and local abort handling is unchanged.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

#27396 deleted the write-only registry that consumed it; nothing sets the
option now, and its doc comment implied a cancellation path that no longer exists.
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 04319c2b-879c-43d4-9f79-1c1a0fa663af

📥 Commits

Reviewing files that changed from the base of the PR and between 57452c6 and 72e1767.


📒 Files selected for processing (2)
  • src/main/providers/local-filesystem-provider.ts
  • src/main/runtime/runtime-file-command-target.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.



📝 Walkthrough

Walkthrough

The local filesystem provider options no longer include onTextSearchSpawn. Runtime setup no longer passes that callback to the provider. The provider's search method now always supplies a no-op onSpawn callback to ripgrep. It continues to receive the authorized-path resolver.


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 72e17

This removes unused runtime search bookkeeping without removing request-abort cancellation. No actionable merge risk remains.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: removing the unused text-search spawn hook.
Description check Passed The description is complete and focused. It explains the change, rationale, linked issue, lack of UI impact, testing, platform considerations, and checklist status.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@nwparker
nwparker merged commit e8b3981 into main Oct 11, 2026
46 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant