feat(lattice): add ShadowFilesystem simulator for the v2 state lattice (closes #655)#668
Conversation
|
@SakethSumanBathini is attempting to deploy a commit to the sreerevanth's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a v2 in-memory ChangesShadow filesystem
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🧪 PR Test Results
Python 3.12 · commit 2c96984 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agentwatch/lattice/shadow_filesystem.py`:
- Around line 94-117: Fix ShadowFilesystem.__init__ so a relative root cannot
produce a duplicated or non-absolute self.root: either validate Path(root) is
absolute and fail clearly, or resolve it against an explicit absolute anchor
before calling _normalise. Ensure known_paths continue to normalize against the
resulting self.root and preserve the existing absolute-root behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 90262a20-af7d-4b2b-bf29-2cfe65c34713
📒 Files selected for processing (3)
agentwatch/lattice/__init__.pyagentwatch/lattice/shadow_filesystem.pytests/test_shadow_filesystem.py
…ntly corrupting it
SHAURYASANYAL3
left a comment
There was a problem hiding this comment.
Looks good, very clean and adheres to V2 constraints. Approving!
Summary
Phase 2 of the v2 state lattice (#651) needs a way to decide whether an agent's filesystem action is
safe by working out what it would do, rather than by pattern-matching the command that requests it.
This adds the filesystem half:
ShadowFilesystem, an in-memory model that answers "what would thisaction do?" without ever touching the disk.
Closes #655
What's here
agentwatch/lattice/shadow_filesystem.py—ShadowFilesystem(root, known_paths)with:simulate(action: FileAction) -> MutationResult— the method the issue asks for. Computes thenormalised target path, the mutation type (
CREATE/MODIFY/DELETE), and whether the target isa critical system path.
apply(action)— simulates and folds the outcome into the model, for reasoning about asequence of actions where a file created in step 1 should read as an overwrite when step 2 writes to
it again. Still no disk access.
exists(path)— whether the shadow tree believes a path exists.MutationResultcarries two fields beyond what the issue lists, because the lattice will need them:existed_before— distinguishes a real delete from a no-op (deleting something that was neverthere), and a fresh write from an overwrite.
escapes_root— whether the target falls outside the workspace root, which is exactly whatcatches something like
../../etc/passwdreaching beyond where the action was scoped to run.CRITICAL_SYSTEM_PATHSmirrors the set already used inagentwatch.core.blast_radius(/etc,/var,/boot,/root,/home,/usr,/bin,/sbin) — checked directly rather than assumed, since adocstring claiming that and being wrong would be worse than not claiming it.
The crucial constraint — proven, not asserted
The issue is explicit that the simulator must never execute real I/O. That's a test, not a comment:
test_no_filesystem_call_is_made_during_simulationwraps every filesystem entry point the class couldplausibly reach (
Path.resolve,.exists,.stat,os.stat,os.listdir,builtins.open, and more)in a recorder and asserts none of them fired while the simulator ran — including against a root that
doesn't exist on disk at all.
The design choice that makes this straightforward: path normalisation goes through
os.path.normpath(pure string work) rather than
Path.resolve(), which stats the filesystem to follow symlinks.A real cross-platform bug, found and fixed before merge
Running the suite on Windows caught something my Linux development environment couldn't:
_normalise()decided absoluteness withPath.is_absolute(), which is platform-dependent for aPOSIX-style path with no drive letter —
PureWindowsPath("/etc/passwd").is_absolute()isFalse. Thatmade
root / pathjoin it as if it were relative, so on Windows/etc/passwdwas silently folded intoa path underneath the workspace root, and both
is_critical_pathandescapes_rootcame back wrong.For a simulator whose entire job is catching exactly this kind of dangerous absolute-path write, that
would have been a silent failure on the platform.
Fixed with a leading-slash string check instead of
Path.is_absolute()— the paths this simulatorreasons about are POSIX paths regardless of what OS the lattice happens to run on. Verified directly
against
PureWindowsPathsemantics (not just re-run and hoped): the old code producedC:\etc\passwdfor that input, the new code produces
\etc\passwd. There's a dedicated regression test for it, and theexisting Linux-side behaviour is unchanged.
Testing
pytest tests/test_shadow_filesystem.py— 34 passed, covering:./..collapsing, traversal out of the rootCRITICAL_SYSTEM_PATHSand its descendants flagged, ordinary paths not flagged, and alookalike prefix (
/etcetera) correctly not flaggedapply()sequencing (create → later write reads as modify; delete removes from the model)simulate()alone leaving the model untouchedpytest tests/(full suite) — 116 failed / 816 passed, identical failure count to a clean runwithout this module (missing optional deps —
celery,redis,litellm,prometheus_client— in theenvironment used to verify this), confirming zero regressions elsewhere.
ruff check,ruff format --check,mypy(on this module) — all clean.bandit— clean.Scope
This is the filesystem shadow model only, matching the issue. Wiring it into the lattice's decision
layer, and any other resource types the lattice needs (network, process, etc.), are follow-up work under
#651.
Summary by CodeRabbit
New Features
Tests