Skip to content

fix(sandboxd): report the changed entry's name in watch events - #670

Merged
xiaods merged 1 commit into
mainfrom
fix/sandboxd-watch-event-name
Oct 22, 2026
Merged

xiaods merged 1 commit into
mainfrom
fix/sandboxd-watch-event-name

Conversation

@xiaods

@xiaods xiaods commented Oct 22, 2026

Copy link
Copy Markdown
Owner

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:

[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.

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.
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

1 024 tests  ±0   997 ✅ ±0   6m 59s ⏱️ -6s
  120 suites ±0    27 💤 ±0 
    1 files   ±0     0 ❌ ±0 

Results for commit 85bb59a. ± Comparison against base commit 2ed56f1.

@xiaods
xiaods merged commit 23afaa2 into main Oct 22, 2026
9 checks passed
@xiaods
xiaods deleted the fix/sandboxd-watch-event-name branch October 27, 2026 06:34
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.

1 participant