Skip to content

fix(runtime): separate dash-prefixed Grep patterns with -- in sandbox worker - #3735

Closed
yunaremaia wants to merge 2 commits into
apache:mainfrom
yunaremaia:fix/grep-dash-pattern-separator
Closed

fix(runtime): separate dash-prefixed Grep patterns with -- in sandbox worker#3735
yunaremaia wants to merge 2 commits into
apache:mainfrom
yunaremaia:fix/grep-dash-pattern-separator

Conversation

@yunaremaia

Copy link
Copy Markdown
Contributor

Summary

  • The sandboxed filesystem worker appended the Grep pattern as a bare positional, so ripgrep parsed a dash-prefixed pattern as flags. A leading flag exits 1, and this worker maps exit 1 to an empty match set - so the model was told a present string was absent, with no error.
  • Adds the -- argv separator the host-local workspace executor already pins (workspace-executor.ts, from feat(windows): add brokered AppContainer sandbox support #2961), plus a regression test asserting the separator position and a successful -webkit-box search.

Test plan

  • New regression test fails without the fix (asserts -- precedes the pattern in the spawned argv) and passes with it
  • Reproduced against real ripgrep 15.1.0: rg -n --no-heading --max-count=5 -webkit-box style.css exits 1; with -- it matches
  • Full runtime suite locally: 3059/3068 pass; the 2 failures are pre-existing root-user permission tests (chmod-restricted files are readable by root, CI runs non-root) - untouched by this diff

Fixes #3733

@M4n5ter
M4n5ter force-pushed the fix/grep-dash-pattern-separator branch 3 times, most recently from 24fc00d to 6515e4d Compare August 26, 2026 09:47
… worker

The filesystem worker appended the pattern as a bare positional, so a
pattern starting with '-' was parsed by ripgrep as flags. A leading flag
exits 1, which maps to an empty match set - reporting strings that exist
as absent, with no diagnostic.

Adds the same '--' separator the host-local workspace executor already
pins (apache#2961), plus a regression test asserting the separator and a
successful dash-prefixed search.

Fixes apache#3733
Desktop e2e failure is the known flake tracked in apache#3727 (slash-command-menu
projection refresh); unrelated to this runtime-only diff.
@M4n5ter

M4n5ter commented Aug 27, 2026

Copy link
Copy Markdown
Member

Relationship note: PRs #3734, #3735, and #3903 all close #3733, touch the same two runtime files, and apply the same -- separator fix to the sandboxed Grep invocation. I could not find a canonical/supersede statement in these PR bodies or comments. At the current snapshot, #3903 is the latest-created candidate, but the owner should decide the canonical PR. Please document that relationship and mark the other attempts as superseded/close them, or explain the distinct scope, so the issue is not merged three times.

This comment records the relationship only; it is not a merge decision.


Automated review note posted by @未开智选手. This is not an independent human review; a human should verify the conclusion.

@M4n5ter

M4n5ter commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thank you for working on this fix. I compared this implementation with #3735 and #3903. All three correctly address the same root cause by placing the user-controlled pattern after ripgrep's -- option terminator.

#3903 was selected because its regression pins both the exact terminal argv order (['--', pattern, path]) and the full worker response with the smallest production patch. It has now been merged as 07e40feed27e748870b8144b3e37bf96c2748c52, so this equivalent implementation is superseded and I am closing it to keep one canonical fix.


Posted by an automated review agent operated by @M4n5ter. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.

简体中文

本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@M4n5ter M4n5ter closed this Aug 27, 2026
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.

bug(runtime): Grep reports "no matches" for any pattern starting with -

2 participants