feat: working fwatcher integration in Runfile#43
Merged
nxtcoder17 merged 1 commit intomasterfrom Apr 24, 2026
Merged
Conversation
Reviewer's GuideImplements a working file-watcher-driven re-run pipeline for Runfile tasks, improves command output layout with Unicode-aware width calculations, adds circular task dependency detection (with tests), tightens shell command cancellation semantics, and fixes a couple of smaller configuration/IO issues and example Runfile entries. Sequence diagram for watcher-driven task rerun pipelinesequenceDiagram
actor User
participant CLI
participant Resolver
participant Watcher
participant CurrentPipeline as Pipeline
User->>CLI: run <task> with watch enabled
CLI->>Resolver: RunTask(ctx, name)
Resolver->>Resolver: createSteps(task)
Resolver->>CurrentPipeline: NewPipeline(logger, steps)
alt watch not enabled
Resolver->>CurrentPipeline: Start(ctx)
CurrentPipeline-->>Resolver: completion or error
Resolver-->>CLI: return
else watch enabled
Resolver->>Watcher: NewWatcher(ctx, WatcherArgs)
Resolver->>Watcher: Watch(ctx)
activate Watcher
Resolver->>Resolver: run()
note over Resolver,CurrentPipeline: run() stops existing pipeline (if any), creates new pipeline and starts it in a goroutine
Resolver->>CurrentPipeline: Start(ctx) (async)
loop file change events
Watcher-->>Resolver: event from GetEvents()
Resolver->>Resolver: reset debounce timer (300ms)
alt debounce expires
Resolver->>Resolver: increment reload counter
Resolver->>Resolver: run() (stop old pipeline, start new)
end
end
alt context cancelled
CLI-->>Resolver: ctx.Done()
Resolver->>CurrentPipeline: Stop()
Resolver->>Watcher: Close()
deactivate Watcher
Resolver-->>CLI: return nil
end
end
Class diagram for task resolution, watching, and pipeline executionclassDiagram
class Resolver {
+RunTask(ctx context.Context, name string) error
+createSteps(task *ResolvedTask, args createCommandGroupArgs) ([]executor.Step, error)
+appendCycle(taskTrail []string, next string) []string
}
class ResolvedTask {
+Name string
+Watch *WatchConfig
}
class WatchConfig {
+Enabled bool
+Dirs []string
+IgnoreDirs []string
+Debounce int
}
class Watcher {
+Watch(ctx context.Context)
+GetEvents() <-chan Event
+Close() error
}
class Event {
+Name string
+Op string
}
class Pipeline {
+Start(ctx context.Context) error
+Stop()
}
class Executor {
+NewPipeline(logger *slog.Logger, steps []Step) *Pipeline
+NewShellCommand(handler func(ctx context.Context) *exec.Cmd) *command
+NewInteractiveShellCommand(handler func(ctx context.Context) *exec.Cmd) *command
}
class command {
+Run(ctx context.Context) error
}
class ShellCommand {
+NewShellCommand(handler func(ctx context.Context) *exec.Cmd) *command
}
class InteractiveShellCommand {
+NewInteractiveShellCommand(handler func(ctx context.Context) *exec.Cmd) *command
}
class Writer {
+PrefixedWriter
+Write(p []byte) (int, error)
+GetStyledPrefix(prefix string) string
}
class LayoutHelpers {
+printCommand(w *writer.LogWriter, prefix string, lang string, cmd string)
+longestLineWidth(str string) int
+padString(str string, withPrefix string) string
+prefixDisplayWidth(prefix string) int
}
Resolver --> ResolvedTask
ResolvedTask --> WatchConfig
Resolver --> Watcher : uses
Resolver --> Pipeline : creates
Executor --> Pipeline : NewPipeline
ShellCommand --> command : returns
InteractiveShellCommand --> command : returns
Executor --> ShellCommand
Executor --> InteractiveShellCommand
Writer <.. LayoutHelpers : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The change in
PrefixedWriter.Writewhereio.EOFnow triggers a direct write to the underlying writer instead of buffering the partial line changes semantics for lines without a trailing newline; this likely breaks the intent of prefixing only complete lines and preserving partial lines across writes, so it would be good to re-evaluate and add a test that covers incremental writes without\n. - In
RunTask, the debounce loop resets the timer on every file event but ignoresev.Namein the reload log; if the intent is to help users see what triggered the reload, consider including the file path in the[RELOADING]log message as before.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change in `PrefixedWriter.Write` where `io.EOF` now triggers a direct write to the underlying writer instead of buffering the partial line changes semantics for lines without a trailing newline; this likely breaks the intent of prefixing only complete lines and preserving partial lines across writes, so it would be good to re-evaluate and add a test that covers incremental writes without `\n`.
- In `RunTask`, the debounce loop resets the timer on every file event but ignores `ev.Name` in the reload log; if the intent is to help users see what triggered the reload, consider including the file path in the `[RELOADING]` log message as before.
## Individual Comments
### Comment 1
<location path="pkg/runfile/resolver/task.go" line_range="165" />
<code_context>
+ dequeueDuration := 300 * time.Millisecond
</code_context>
<issue_to_address>
**issue (bug_risk):** Timer debouncing logic can misbehave due to Reset being called on an active timer without Stop/drain
`time.Timer`’s contract requires `Reset` to be called only on a stopped or fully-drained timer. Resetting it while it may still be active can cause races and extra or delayed firings, especially under bursty events.
Consider refactoring to a pattern that:
- Lazily creates the timer.
- Calls `Stop()` and drains `timer.C` (if needed) before each `Reset`.
- Uses a separate `timerC` channel variable in the `select` to avoid reading from an uninitialized or already-fired timer.
This will make the debounce behavior deterministic and avoid subtle timing bugs.
</issue_to_address>
### Comment 2
<location path="pkg/writer/writer.go" line_range="27-28" />
<code_context>
line, err := pw.buf.ReadBytes('\n')
if errors.Is(err, io.EOF) {
pw.buf.Reset()
- pw.buf.Write(pw.render(line))
+ pw.w.Write(pw.render(line))
break
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Change in handling of partial lines on Write likely alters prefixing behavior and can cause duplicated prefixes
Previously, on `io.EOF` the code kept the *rendered* (prefixed) partial line in `pw.buf`, so the same logical line was only rendered/prefixed once when completed. Now the partial line is written directly to `pw.w` and the buffer is cleared, so a line split across writes will be prefixed multiple times, e.g.
```text
[PREFIX] hell
[PREFIX] o world
```
If you want to keep a single prefix per logical line while supporting streaming, consider buffering raw bytes until a newline is seen and only rendering/writing once the line is complete, or explicitly accept and document the new behavior of re-prefixed partial lines.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
27
to
-28
| pw.buf.Reset() | ||
| pw.buf.Write(pw.render(line)) |
There was a problem hiding this comment.
issue (bug_risk): Change in handling of partial lines on Write likely alters prefixing behavior and can cause duplicated prefixes
Previously, on io.EOF the code kept the rendered (prefixed) partial line in pw.buf, so the same logical line was only rendered/prefixed once when completed. Now the partial line is written directly to pw.w and the buffer is cleared, so a line split across writes will be prefixed multiple times, e.g.
[PREFIX] hell
[PREFIX] o world
If you want to keep a single prefix per logical line while supporting streaming, consider buffering raw bytes until a newline is seen and only rendering/writing once the line is complete, or explicitly accept and document the new behavior of re-prefixed partial lines.
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
1 participant
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.
Summary by Sourcery
Improve task watching, command execution robustness, and cycle detection in Runfile tasks.
New Features:
test-watchexample task demonstrating watch-based execution in the example Runfile.Bug Fixes:
Enhancements:
Tests:
Chores: