Skip to content

Accept a predicate for forbidden_read / forbidden_write - #29

Merged
shellygr merged 1 commit into
masterfrom
shelly/vfs-forbidden-predicates
Aug 4, 2026
Merged

Accept a predicate for forbidden_read / forbidden_write#29
shellygr merged 1 commit into
masterfrom
shelly/vfs-forbidden-predicates

Conversation

@shellygr

@shellygr shellygr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

global_exclude accepts either a fullmatch regex or a Callable[[PurePath], bool]
(GlobalExcludeArg), but forbidden_read / forbidden_write accept only a regex. The
comment on the type alias already called out why that asymmetry hurts:

the regex form is retained for symmetry with forbidden_read/forbidden_write but is
clunky for path logic (you end up writing (.*/)?node_modules(/.*)? for what's just
"node_modules" in p.parts).

Two rules a consumer actually wants are painful or unreadable as a regex:

  • exclude a directory at any depth(?:.*/)?node_modules/.* versus
    "node_modules" in p.parts
  • carve one file kind back out of an excluded subtree — needs a lookahead
    ((?!.*\.sol\Z)(?:…)) to say "withhold this tree, but never Solidity"

Change

  • forbidden_read / forbidden_write widen from str | None to GlobalExcludeArg, on
    VFSToolConfig, fs_tools and fs_tools_layered.
  • Both surface forms are now interpreted in exactly one place, _make_exclude_pred.
    _make_checker and _make_global_include_pred are thin wrappers over it that differ
    only in whether the always-on .git floor is composed in — previously each re-parsed
    the argument itself.

The regex arm stays, since it is the right shape for a pattern supplied as a CLI
argument. Callables are handed a PurePosixPath, so parts / suffix / name behave
the same regardless of host platform.

No behaviour change for existing callers: a str argument follows the identical
re.fullmatch path it did before.

Testing

  • 3 new tests in TestForbiddenRead: the callable form filters the tool surface, can
    express a carve-out (suffix != ".sol" and "node_modules" in parts) that a plain regex
    cannot state without a lookahead, and receives a PurePosixPath.
  • Full VFS suite green: 72 passed (tests/test_vfs.py, tests/test_fs_layered.py).
  • pyright clean on graphcore/ and tests/ (two pre-existing testcontainers.postgres
    import errors in conftest.py are unchanged).

🤖 Generated with Claude Code

These two tool filters took a fullmatch regex while global_exclude already
accepted either a regex or a Callable[[PurePath], bool]. A regex is the wrong
shape for anything that reasons about path structure: excluding a directory at
any depth reads as (.*/)?node_modules/.* rather than "node_modules" in p.parts,
and carving one file kind back out of an excluded subtree needs a lookahead.

Widen both to GlobalExcludeArg and fold the two surface forms into a single
_make_exclude_pred, so the regex and callable arms are interpreted in one place
and _make_checker / _make_global_include_pred differ only by the .git floor.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@shellygr
shellygr marked this pull request as ready for review August 4, 2026 08:44
@shellygr
shellygr merged commit c8b3ae5 into master Aug 4, 2026
2 checks passed
@shellygr
shellygr deleted the shelly/vfs-forbidden-predicates branch August 4, 2026 08:45
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