Skip to content

feat(agent/agentcontainers): add file watcher and dirty status #17573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Apr 29, 2025
Merged

feat(agent/agentcontainers): add file watcher and dirty status #17573

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
40c8de9
feat(agent/agentcontainers): add file watcher and dirty status
mafredri Apr 28, 2025
a997f82
make gen
mafredri Apr 28, 2025
0e568fb
fix copilot nits
mafredri Apr 28, 2025
4f788d9
log event string in case not write
mafredri Apr 28, 2025
bfeb1da
fix chmod on macos
mafredri Apr 28, 2025
45a978c
fix close of closed channel
mafredri Apr 28, 2025
d2721a7
add close protection to fsnotify watcher as well
mafredri Apr 28, 2025
c03bb7d
fix typo
mafredri Apr 28, 2025
aed7c30
fix pr suggestions and improve start loop
mafredri Apr 28, 2025
39526e6
update noop impl.
mafredri Apr 28, 2025
7edee90
s/file/path
mafredri Apr 28, 2025
9cf5415
rewrite watcher impl. to watch dirs
mafredri Apr 28, 2025
de2a42e
add rename and create to fsnotify test
mafredri Apr 28, 2025
0cbee38
add atomic file replacement
mafredri Apr 28, 2025
5ced7e1
add paranoid reset
mafredri Apr 28, 2025
5a96a55
fix oopsie
mafredri Apr 28, 2025
0b16448
s/ErrWatcherClosed/ErrClosed/g
mafredri Apr 28, 2025
eaf5922
inverse continue/return for clarity
mafredri Apr 28, 2025
617fed2
better close handling
mafredri Apr 28, 2025
8ee9914
restore removed comment
mafredri Apr 28, 2025
263c683
add note about name
mafredri Apr 28, 2025
58a70e8
fix test setup/clock advancement
mafredri Apr 28, 2025
9f739ca
outdent
mafredri Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
s/ErrWatcherClosed/ErrClosed/g
  • Loading branch information
@mafredri
mafredri committed Apr 28, 2025
commit 0b16448052173f8a890bec005578ea1c8a3e018b
2 changes: 1 addition & 1 deletion agent/agentcontainers/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (api *API) start() {
for {
event, err := api.watcher.Next(api.ctx)
if err != nil {
if errors.Is(err, watcher.ErrWatcherClosed) {
if errors.Is(err, watcher.ErrClosed) {
api.logger.Debug(api.ctx, "watcher closed")
return
}
Expand Down
4 changes: 2 additions & 2 deletions agent/agentcontainers/watcher/noop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ func (n *noopWatcher) Next(ctx context.Context) (*fsnotify.Event, error) {
case <-ctx.Done():
return nil, ctx.Err()
case <-n.done:
return nil, ErrWatcherClosed
return nil, ErrClosed
}
}

func (n *noopWatcher) Close() error {
n.mu.Lock()
defer n.mu.Unlock()
if n.closed {
return ErrWatcherClosed
return ErrClosed
}
n.closed = true
close(n.done)
Expand Down
10 changes: 5 additions & 5 deletions agent/agentcontainers/watcher/watcher.go
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this package could be moved outside of agentcontainers for later re-use, but that doesn't have to be done in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a valid suggestion, but I'd like to defer until the need arises to keep devcontainer related stuff contained (heh).

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"golang.org/x/xerrors"
)

var ErrWatcherClosed = xerrors.New("watcher closed")
var ErrClosed = xerrors.New("watcher closed")

// Watcher defines an interface for monitoring file system changes.
// Implementations track file modifications and provide an event stream
Expand Down Expand Up @@ -140,7 +140,7 @@ func (f *fsnotifyWatcher) Next(ctx context.Context) (event *fsnotify.Event, err
return nil, ctx.Err()
case evt, ok := <-f.Events:
if !ok {
return nil, ErrWatcherClosed
return nil, ErrClosed
}

// Get the absolute path to match against our watched files.
Expand All @@ -160,11 +160,11 @@ func (f *fsnotifyWatcher) Next(ctx context.Context) (event *fsnotify.Event, err

case err, ok := <-f.Errors:
if !ok {
return nil, ErrWatcherClosed
return nil, ErrClosed
}
return nil, xerrors.Errorf("watcher error: %w", err)
case <-f.done:
return nil, ErrWatcherClosed
return nil, ErrClosed
}
}
}
Expand All @@ -178,7 +178,7 @@ func (f *fsnotifyWatcher) Close() (err error) {
f.mu.Unlock()

if closed {
return ErrWatcherClosed
return ErrClosed
}

close(f.done)
Expand Down
Loading