Repository navigation
fix(sandboxd): report the changed entry's name in watch events - #670
Merged
Merged
Conversation
Running the K8E guide's Level 4 example against the cluster showed the
watch event carrying the watched directory instead of the file that
changed:
[PASS] Level 4 — files.watch_dir (polling trio):
guide form -> TypeError (guide snippet bug)
get_new_events() -> [('/workspace//', 'FilesystemEventType.CREATE')]
The guide expects `event.name` to be "a.txt".
Two bugs, both in sandboxd/src/watch.zig:
1. The inotify record was mis-parsed. `inotify_event.len` is the length
of the NAME — including its terminating NUL, padded so the record
stays 4-byte aligned — not the length of the record. The code treated
it as the record length, so:
- the name slice was computed as buf[pos+16 .. pos+len-17], which
collapses to the empty string for any name of 15 bytes or less.
"a.txt" is 5 bytes, so every event reported an empty name;
- the cursor advanced by len instead of by the record length, which
desynchronised the event stream after the first record.
The record length is header + len, and the name ends at the first
NUL — len counts the NUL and the alignment padding, so the name is
not len-1 bytes long.
2. The event name was the joined absolute path. The SDK documents
FilesystemEvent.name as the "relative path to the filesystem object"
— relative to the watched directory — which is exactly what inotify
hands over as the entry name. The SDK matches the name against the
entries it listed in that directory, so the absolute path was wrong
twice over, and the join produced a doubled slash when the name was
empty.
The parsing is now a pure function, parseInotifyRecords(), so it can be
unit-tested without an inotify fd.
Also: sandboxd/src/watch_test.zig was never wired into `zig build test`
— build.zig listed exec/files/transcript/events/pty/httpio but not
watch, so the two watch tests that existed had never run. Adding the
module brings the file in and the four new cases with it.
Verified against the guide's own Level 4 example:
before: get_new_events() -> [('/workspace//', 'CREATE')]
after: get_new_events() -> [('a.txt', 'CREATE'), ('a.txt', 'WRITE')]
sandboxd: 67/67 tests pass (was 61: 4 new watch cases + the 2 that
had never been collected). The four new cases were checked against the
old parser — three of them fail without the fix.
Full E2B SDK regression on the cluster: Python 44/44, JavaScript 17/17.
The guide's Level 1-5 examples pass on both languages.
|
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



fix(sandboxd): report the changed entry's name in watch events
Running the K8E guide's Level 4 example against the cluster showed the
watch event carrying the watched directory instead of the file that
changed:
The guide expects
event.nameto be "a.txt".Two bugs, both in sandboxd/src/watch.zig:
The inotify record was mis-parsed.
inotify_event.lenis the lengthof the NAME — including its terminating NUL, padded so the record
stays 4-byte aligned — not the length of the record. The code treated
it as the record length, so:
collapses to the empty string for any name of 15 bytes or less.
"a.txt" is 5 bytes, so every event reported an empty name;
desynchronised the event stream after the first record.
The record length is header + len, and the name ends at the first
NUL — len counts the NUL and the alignment padding, so the name is
not len-1 bytes long.
The event name was the joined absolute path. The SDK documents
FilesystemEvent.name as the "relative path to the filesystem object"
— relative to the watched directory — which is exactly what inotify
hands over as the entry name. The SDK matches the name against the
entries it listed in that directory, so the absolute path was wrong
twice over, and the join produced a doubled slash when the name was
empty.
The parsing is now a pure function, parseInotifyRecords(), so it can be
unit-tested without an inotify fd.
Also: sandboxd/src/watch_test.zig was never wired into
zig build test— build.zig listed exec/files/transcript/events/pty/httpio but not
watch, so the two watch tests that existed had never run. Adding the
module brings the file in and the four new cases with it.
Verified against the guide's own Level 4 example:
sandboxd: 67/67 tests pass (was 61: 4 new watch cases + the 2 that
had never been collected). The four new cases were checked against the
old parser — three of them fail without the fix.
Full E2B SDK regression on the cluster: Python 44/44, JavaScript 17/17.
The guide's Level 1-5 examples pass on both languages.