Skip to content

fix(harness): exclude shadowed lower grep matches in OverlayFilesystem - #2450

Open
Blue-Berrys wants to merge 1 commit into
agentscope-ai:mainfrom
Blue-Berrys:fix/2448-overlay-grep-upper-override
Open

fix(harness): exclude shadowed lower grep matches in OverlayFilesystem#2450
Blue-Berrys wants to merge 1 commit into
agentscope-ai:mainfrom
Blue-Berrys:fix/2448-overlay-grep-upper-override

Conversation

@Blue-Berrys

@Blue-Berrys Blue-Berrys commented Jul 28, 2026

Copy link
Copy Markdown

Summary

  • Fix OverlayFilesystem.grep so upper-layer files fully shadow lower-layer matches for the same path, even when the upper file no longer contains the pattern.
  • Add regression tests covering upper-edit-removes-match and upper-match-overrides-lower-match.

Closes #2448

Description

OverlayFilesystem documents that upper entries override lower entries on path collision, but grep previously merged by path:line only. When an upper-layer file existed at the same path but no longer matched, the lower-layer hit remained because upperResult had no entry for that path.

The fix checks upper.exists(...) for each lower match path and skips shadowed lower matches before merging upper hits.

Checklist

  • Code has been formatted with mvn spotless:apply
  • Targeted tests pass (OverlayFilesystemGrepTest)
  • Related documentation has been updated (N/A)
  • Code is ready for review

Testing

JAVA_HOME=/path/to/jdk21 mvn -pl agentscope-harness -am \
  -Dspotless.check.skip=true \
  -Dtest=OverlayFilesystemGrepTest \
  -Dsurefire.failIfNoSpecifiedTests=false \
  test

When an upper-layer file exists at the same path, lower-layer grep hits for
that path must not be returned even if the upper file no longer matches.
Closes agentscope-ai#2448.
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@CLAassistant

CLAassistant commented Jul 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review — Approved ✅

Clean fix for the stale lower-layer grep match issue in OverlayFilesystem.

Code analysis:

  • The shadowing logic correctly uses upper.exists() to determine whether a lower-layer path is fully overridden, regardless of whether the upper file still matches the pattern.
  • upperShadowedPaths set avoids redundant upper.exists() calls for duplicate paths in lower matches.
  • Regression tests cover both key scenarios: upper-edit-removes-match and upper-match-overrides-lower.

Minor note: For very large grep result sets, the per-path upper.exists() calls could add up. Consider batching if this becomes a bottleneck in practice — but not a blocker.

One nit: the upperShadowedPaths.contains(matchPath) check on line 217 is redundant after the block above already adds to the set and continues. Could simplify to just the upper.exists() check + add + continue pattern.

LGTM. Please sign the CLA when you get a chance.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...pe/harness/agent/filesystem/OverlayFilesystem.java 87.50% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@Blue-Berrys
Blue-Berrys force-pushed the fix/2448-overlay-grep-upper-override branch from 4a86955 to ce8352f Compare July 28, 2026 08:08
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]: OverlayFilesystem.grep keeps stale lower-layer matches after upper edits

3 participants