Skip to content

fix(cli): safely restore grep signal controls - #12882

Merged
marius-kilocode merged 3 commits into
mainfrom
reintroduce-optimization-with-agent-safety-tests
Aug 6, 2026
Merged

fix(cli): safely restore grep signal controls#12882
marius-kilocode merged 3 commits into
mainfrom
reintroduce-optimization-with-agent-safety-tests

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

PR #12811 added configurable grep context, limits, literal matching, and case-insensitive matching, but bounded searches could leave an agent waiting after the direct ripgrep process had already exited. PR #12847 reverted the controls because descendants retaining inherited output pipes could delay Node's close event indefinitely.

This restores the signal controls while making early settlement an explicit ripgrep-only process policy. Bounded ripgrep commands settle on direct process exit, clean up owned process-group descendants on POSIX, and retain force-kill escalation for cancellation. Other commands keep the existing close-based lifecycle, so ordinary background shell processes are not pulled into this behavior.

The regression coverage recreates the inherited-pipe condition with a platform-specific fake ripgrep executable. The existing unit matrix exercises settlement and cancellation on Linux, macOS, and Windows; POSIX jobs additionally assert owned process-group cleanup while detached descendants remain untouched.

Comment thread packages/core/src/cross-spawn-spawner.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The incremental commit renames the spawn helper's third parameter from settle: boolean to direct: boolean, resolving the previous shadowing finding. The rename is consistent across the declaration (line 275) and its use (line 288), the single call site passes the flag positionally, and the settle import from ./kilocode/exit-code is no longer shadowed in that scope.

Files Reviewed (1 file)
  • packages/core/src/cross-spawn-spawner.ts
Previous Review Summary (commit 348db37)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 348db37)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/core/src/cross-spawn-spawner.ts 272 settle boolean parameter shadows the settle import from ./kilocode/exit-code

The core settlement design checks out: bounded ripgrep commands settle on direct process exit instead of close, POSIX process-group cleanup is scoped to direct-settling commands and leaves detached descendants alone, force-kill escalation still bounds cancellation, and the regression tests recreate the inherited-pipe condition faithfully. Context/match accounting in stop/select is consistent, and the Kilo-specific logic is properly extracted into kilocode/ mirror files with markers on shared code.

Files Reviewed (12 files)
  • .changeset/green-greps-settle.md
  • packages/core/src/cross-spawn-spawner.ts - 1 issue
  • packages/core/src/kilocode/ripgrep-grep.ts
  • packages/core/src/kilocode/spawn-exit.ts
  • packages/core/src/ripgrep.ts
  • packages/core/test/kilocode/ripgrep-settlement.test.ts
  • packages/opencode/src/kilocode/tool/grep-signal-controls.ts
  • packages/opencode/src/tool/grep.ts
  • packages/opencode/test/kilocode/tool/grep-signal-controls.test.ts
  • packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap
  • packages/opencode/test/tool/grep.test.ts
  • packages/opencode/test/tool/parameters.test.ts

Fix these issues in Kilo Cloud


Reviewed by kimi-k3 · Input: 34.1K · Output: 3.7K · Cached: 235.5K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode merged commit 81ddf2d into main Aug 6, 2026
31 checks passed
@marius-kilocode
marius-kilocode deleted the reintroduce-optimization-with-agent-safety-tests branch August 6, 2026 08:01
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…ion-with-agent-safety-tests

fix(cli): safely restore grep signal controls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants