Feat/embed daemon#43
Conversation
- Added binaries to .gitignore to prevent tracking of build artifacts. - Updated CHANGELOG to include new features and changes for version 0.7.0, highlighting cross-platform support, backend cross-compilation, and improvements in port conflict detection and UI build targets. - Enhanced documentation in CLAUDE.md and ROADMAP.md to reflect the new daemon integration and installation plans. - Updated Makefile to include new build targets for Linux and Windows. - Bumped version to 0.7.0 in backend and UI configurations.
- Updated CHANGELOG to document changes for version 0.7.0, including fixes and enhancements. - Modified Makefile to include new build targets for macOS, Linux, and Windows. - Removed unnecessary entitlements from macOS release configuration to comply with app sandboxing requirements. - Refactored container parsing logic to unify handling of labels and improve error handling in the backend. - Enhanced UI logic to ensure proper state management during daemon startup.
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR ships v0.7.0: platform-specific build and release targets, embedded daemon startup and lifecycle handling, Lima Docker CLI proxying, shared UI port opening, a removed settings toggle, and nerdctl container parsing fixes. ChangesBuild & Release Infrastructure
Daemon Embedding & Docker Runtime
Estimated code review effort: 5 (Critical) | ~120 minutes UI Port Opening Refactor & Settings Cleanup
Estimated code review effort: 2 (Simple) | ~15 minutes Container List Parsing Fix
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant main as main()
participant App as _MainAppState
participant Daemon as calf-daemon
participant API as v1/status
main->>Daemon: start daemon process
main->>App: runApp()
App->>App: initState() calls _waitForDaemon()
loop poll until ready or timeout
App->>API: GET /v1/status
API-->>App: runtime.state
end
App->>App: setState(_daemonReady = true)
App->>App: build() renders AppShell
App->>Daemon: stop() on detached/dispose
sequenceDiagram
participant Lima
participant UnixSocket as DockerCLISocket
participant Proxy as TCPProxy
participant VM as LimaVM
Lima->>Lima: Start()
Lima->>UnixSocket: ensureDockerCLISocket()
UnixSocket->>Proxy: serveTCPProxy() to 127.0.0.1:2375
VM-->>Proxy: forwarded Docker socket via socat
Lima->>Lima: Stop()
Lima->>Proxy: removeDockerCLISocket()
Proxy-->>UnixSocket: close listener/server, remove socket
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
backend/internal/runtime/nerdctl.go (2)
186-221: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
parseLabelslogic is sound; one edge case to consider.The function correctly handles:
- Empty/null labels →
nil- JSON object form →
map[string]string- Comma-separated string form → split on
,thenSplitN("=", 2)One edge case: if a label value contains a comma (e.g.,
key=val,ue), the comma-separated parser will split it into two segments (key=valandue), andue(no=) is silently dropped. This is an inherent limitation of the comma-separated format and likely acceptable for nerdctl's output, but worth documenting with a brief comment if nerdctl can produce such values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/nerdctl.go` around lines 186 - 221, Add a brief comment in parseLabels to document the limitation of the comma-separated parsing path: values containing commas will be split and later fragments without '=' are ignored. Keep the existing logic in parseLabels unchanged, but make the edge case explicit near the strings.Split(s, ",") / SplitN handling so future readers know this behavior is intentional and tied to nerdctl output.
37-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
nerdctlLinestruct.ParseContainerLinesnow uses an inline struct instead, so this type only adds drift and confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/nerdctl.go` around lines 37 - 51, Remove the unused nerdctlLine struct from the runtime parsing code, since ParseContainerLines now relies on an inline struct and this standalone type is no longer referenced. Clean up the type definition in nerdctl.go and make sure any remaining parsing logic continues to use the inline struct or another shared type only if it is actually needed elsewhere.Source: Learnings
.cursor/rules/calf.mdc (1)
147-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
calf.mdcis drifting fromCLAUDE.md— sync themain.goandmain.dartdescriptions.The Makefile entry (line 147) and build commands (lines 255-266) match
CLAUDE.md, but two other updated sections inCLAUDE.mdwere not mirrored here:
- Line 54 —
CLAUDE.mdupdated themain.gotree entry to"...stale-port takeover (cross-platform: lsof/Unix, netstat/Windows)";calf.mdcstill has the old shorter description.- Line 212 —
CLAUDE.mdexpanded themain.dartreference to cover_startDaemon(),/v1/statuspolling, daemon kill-on-close, and macOS PATH injection;calf.mdcstill has the old theme-only description.- Line 12 (CLAUDE.md) — new paragraph about the embedded daemon binary in the
.appbundle has no equivalent incalf.mdc's Project section.Based on learnings,
CLAUDE.mdand.cursor/rules/calf.mdcshould be kept equivalent.Also applies to: 255-266, 295-295
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.cursor/rules/calf.mdc at line 147, Sync calf.mdc with CLAUDE.md by updating the outdated documentation entries for main.go and main.dart to match the newer descriptions. Specifically, adjust the main.go tree entry in the Project section to mention stale-port takeover with cross-platform lsof/Unix and netstat/Windows behavior, and expand the main.dart reference to include _startDaemon(), /v1/status polling, daemon kill-on-close, and macOS PATH injection. Also add the missing Project section note about the embedded daemon binary inside the .app bundle so calf.mdc stays equivalent to CLAUDE.md.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cmd/calf/main.go`:
- Around line 219-222: The listener scan in main should not use a loose
strings.Contains(line, target) check because it can match the wrong socket
entry. Update the matching logic in the loop that processes strings.Split(out,
"\n") so it anchors the port check to the local-address field parsed from
fields, rather than any substring in the full line, and keep the LISTENING
filter tied to the same entry. Use the existing target construction and the
fields slice in this block to ensure the PID lookup only selects the exact port
listener.
- Around line 64-80: The parent-death watchdog in the anonymous goroutine using
os.Getppid() only works on Unix, so it will not detect Flutter parent exit on
Windows. Update the watchdog logic in main.go to use a Windows-specific
parent-exit mechanism when running on Windows, such as tracking a parent process
handle or job object, while keeping the existing Unix path intact. Make the
platform-specific check explicit around the current ticker loop and stop() call
so the daemon shuts down reliably across OSes.
In `@backend/internal/runtime/lima.go`:
- Around line 158-162: The status response is leaking the daemon’s full PATH
through handleStatus by copying os.Getenv("PATH") into status.Log. Keep the
detailed PATH only in the existing limaLogger.Warn call, and change the
user-facing status.Log message to a sanitized limactl-not-found error that does
not include internal environment values. Use the handleStatus flow and the
limactl lookup block as the place to update the returned status payload.
- Around line 739-780: The docker proxy lifecycle in Lima is racing because
serveTCPProxy and removeDockerCLISocket access dockerProxy and proxyListener
from different goroutines without synchronization. Protect all reads and writes
to both fields with l.mu, build the http.Server before starting Serve, and
ensure Stop/removeDockerCLISocket cannot observe or leave behind a stale server
or listener reference.
In `@backend/internal/runtime/lima.yaml`:
- Around line 86-104: The docker-tcp-proxy service currently listens on all
guest interfaces via the Docker proxy setup in the lima.yaml systemd unit, which
exposes the Docker API more broadly than needed. Update the ExecStart for
docker-tcp-proxy to bind the socat TCP listener to loopback only so it serves
just the 127.0.0.1 forward, keeping the existing docker-tcp-proxy service and
portForwards configuration otherwise unchanged.
In `@CHANGELOG.md`:
- Around line 12-33: Rewrite the affected bullets in CHANGELOG.md to describe
user-visible behavior only, removing internal implementation terms like
httputil.ReverseProxy, lsof, netstat, limactl, nerdctl, docker, and any
protocol/tooling jargon. Keep the existing symbols/sections (Cross-platform
support, Changed, Fixed) but restate each item in plain user-facing language
that explains what improved for Linux, macOS, Windows, build/release, port
conflict detection, PATH setup, UI opening, daemon discovery, startup logging,
entitlements, container parsing, streaming/exec, and startup spinner behavior.
In `@CLAUDE.md`:
- Line 12: The bundle path description is incorrect: the daemon is not
`Contents/MacOS/calf` but `Contents/MacOS/calf-daemon`. Update the documentation
text in CLAUDE.md to reference the daemon binary by its actual bundled name, and
keep `Contents/MacOS/calf` reserved for the main Flutter executable so the
launch/cleanup description matches the packaging behavior.
In `@Makefile`:
- Around line 51-54: The release-linux target is building the daemon with the
host OS instead of a Linux target, so update the backend build step in
release-linux to explicitly set GOOS=linux and GOARCH=amd64, matching the
release-windows pattern. Keep the existing ui-linux dependency and copy/remove
steps, but ensure the calf-daemon binary produced by the backend build is always
a Linux binary before it is placed into the ui/build/linux/x64/release/bundle
bundle.
In `@ui/lib/main.dart`:
- Line 194: The try/catch in the daemon startup flow is swallowing all failures
in `main.dart`, which hides real bugs behind the expected “daemon not up yet”
path. Update the catch around the connection check to handle only the known
connection/timeout cases explicitly, and use the surrounding startup logic to
log or surface any unexpected exceptions instead of discarding them. Keep the
fix localized to the code path that probes the daemon so the expected retry
behavior stays intact while bad responses and JSON/parsing failures are visible.
- Around line 59-69: Update _findDaemon so it also resolves the Windows daemon
name fallback by checking for calf-daemon.exe in addition to calf-daemon, using
Platform.resolvedExecutable and the existing daemonPath logic. Remove or revise
the macOS .app bundle comment unless _findDaemon actually performs a
bundle-specific lookup, so the comments match the behavior in this helper.
- Around line 48-53: The daemon restart logic in the _daemonProcess.exitCode
callback can still schedule _startDaemon after shutdown and can also loop
endlessly on repeated startup failures. Add a shutdown guard in the widget/state
that is checked before calling Future.delayed and _startDaemon, and clear or
cancel any pending restart when dispose() or detached runs. Also update the
restart path around _startDaemon to use bounded retries with backoff and a stop
condition so failure states surface instead of continuously respawning the
daemon.
- Line 165: The startup status probe in main.dart is hardcoding the daemon URL
instead of using the shared configured base address. Update the status check in
the startup flow to build the endpoint from the same base URL source used by the
daemon listen address/config, so changes to the port or host stay in sync;
locate the probe near the url constant in main.dart and replace the duplicated
literal with the shared base URL path for v1/status.
---
Nitpick comments:
In @.cursor/rules/calf.mdc:
- Line 147: Sync calf.mdc with CLAUDE.md by updating the outdated documentation
entries for main.go and main.dart to match the newer descriptions. Specifically,
adjust the main.go tree entry in the Project section to mention stale-port
takeover with cross-platform lsof/Unix and netstat/Windows behavior, and expand
the main.dart reference to include _startDaemon(), /v1/status polling, daemon
kill-on-close, and macOS PATH injection. Also add the missing Project section
note about the embedded daemon binary inside the .app bundle so calf.mdc stays
equivalent to CLAUDE.md.
In `@backend/internal/runtime/nerdctl.go`:
- Around line 186-221: Add a brief comment in parseLabels to document the
limitation of the comma-separated parsing path: values containing commas will be
split and later fragments without '=' are ignored. Keep the existing logic in
parseLabels unchanged, but make the edge case explicit near the strings.Split(s,
",") / SplitN handling so future readers know this behavior is intentional and
tied to nerdctl output.
- Around line 37-51: Remove the unused nerdctlLine struct from the runtime
parsing code, since ParseContainerLines now relies on an inline struct and this
standalone type is no longer referenced. Clean up the type definition in
nerdctl.go and make sure any remaining parsing logic continues to use the inline
struct or another shared type only if it is actually needed elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a8491d9a-c40a-4a32-851d-e49610522c33
📒 Files selected for processing (24)
.cursor/rules/calf.mdc.github/workflows/ci.yml.gitignoreCHANGELOG.mdCLAUDE.mdMakefileROADMAP.mdbackend/api.testbackend/buildstore.testbackend/cmd/calf/main.gobackend/internal/runtime/lima.gobackend/internal/runtime/lima.yamlbackend/internal/runtime/nerdctl.gobackend/internal/runtime/runtime.gobackend/runtime.testbackend/version/version.goui/lib/app_shell.dartui/lib/main.dartui/lib/platform/open_url.dartui/lib/screens/compose_group_detail_screen.dartui/lib/screens/container_detail_screen.dartui/lib/screens/containers_screen.dartui/macos/Runner/Release.entitlementsui/pubspec.yaml
💤 Files with no reviewable changes (1)
- ui/lib/app_shell.dart
| go func() { | ||
| ticker := time.NewTicker(5 * time.Second) | ||
| defer ticker.Stop() | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-ticker.C: | ||
| if os.Getppid() == 1 { | ||
| logger.Warn("parent process died, shutting down") | ||
| stop() | ||
| return | ||
| } | ||
| } | ||
| } | ||
| }() | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Windows detect parent process exit Go child process orphan job object
💡 Result:
To detect the termination of a parent process or prevent orphaned child processes in Go on Windows, you can utilize the Windows API via the syscall or golang.org/x/sys/windows packages. To detect if a parent process has exited, the most reliable approach is to obtain a process handle for the parent and wait for it to signal. Since OpenProcess can succeed even if a process has exited (if other handles exist), you must correctly handle synchronization: 1. Obtain a handle to the parent process with SYNCHRONIZE access: h, err:= windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(ppid)) 2. Use WaitForSingleObject to wait on that handle: windows.WaitForSingleObject(h, windows.INFINITE) 3. For an authoritative check that the process has actually terminated rather than just being in a zombie-like state, verify the exit code is no longer STILL_ACTIVE (259): var exitCode uint32 windows.GetExitCodeProcess(h, &exitCode) To prevent orphan processes, you should use Job Objects. Job Objects allow you to group processes and enforce termination policies. Specifically, the JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag causes all processes associated with the job to be automatically terminated when the last handle to the job object is closed. Steps to implement this in Go: 1. Create a job object: Use windows.CreateJobObject(nil, nil). 2. Configure the termination policy: Use windows.SetInformationJobObject with the JOBOBJECT_EXTENDED_LIMIT_INFORMATION structure, setting the BasicLimitInformation.LimitFlags field to JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. 3. Assign child processes to the job: Use windows.AssignProcessToJobObject after starting the child process. By grouping child processes into such a job object, you ensure that if your parent Go process crashes or exits, the OS will automatically clean up the orphaned child processes [1][2]. Top results: [3][1][2]
Citations:
- 1: https://devblogs.microsoft.com/oldnewthing/20131209-00/?p=2433
- 2: https://stackoverflow.com/questions/53208/how-do-i-automatically-destroy-child-processes-in-windows
- 3: knewstimek/agent-tool@b9eac2a
Parent-death watchdog is Unix-only. os.Getppid() == 1 depends on Unix re-parenting to PID 1, so this goroutine never fires on Windows and the daemon can be orphaned when the Flutter parent exits. Add a Windows-specific parent-exit mechanism, such as a parent process handle or job object.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/cmd/calf/main.go` around lines 64 - 80, The parent-death watchdog in
the anonymous goroutine using os.Getppid() only works on Unix, so it will not
detect Flutter parent exit on Windows. Update the watchdog logic in main.go to
use a Windows-specific parent-exit mechanism when running on Windows, such as
tracking a parent process handle or job object, while keeping the existing Unix
path intact. Make the platform-specific check explicit around the current ticker
loop and stop() call so the daemon shuts down reliably across OSes.
…nhancements - Revised CHANGELOG to reflect new features and improvements for version 0.7.0, including cross-platform support and automated builds. - Updated CLAUDE.md to clarify the daemon's integration within the Flutter app bundle. - Modified Makefile to ensure proper cross-compilation for Linux and Windows targets. - Enhanced backend logic for port conflict detection and improved error handling during daemon startup. - Implemented a watchdog mechanism for monitoring the parent process across platforms.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cmd/calf/watchdog_windows.go`:
- Around line 20-31: The goroutine in watchdog_windows.go leaks and can race
with CloseHandle because it waits forever in WaitForSingleObject(handle,
windows.INFINITE) without observing ctx. Update the watchdog logic in the
shutdown watcher function to use a finite-timeout polling loop that periodically
calls WaitForSingleObject with a short timeout, checks ctx.Done() between polls,
and exits cleanly on cancellation. Keep the existing stop() behavior when the
parent process is detected, but make the inner goroutine context-stoppable so it
cannot outlive the outer function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc9319fc-1d9d-4f5a-ba34-6db348dffb88
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
.cursor/rules/calf.mdcCHANGELOG.mdCLAUDE.mdMakefilebackend/cmd/calf/main.gobackend/cmd/calf/watchdog_unix.gobackend/cmd/calf/watchdog_windows.gobackend/go.modbackend/internal/runtime/lima.gobackend/internal/runtime/lima.yamlbackend/internal/runtime/nerdctl.goui/lib/main.dart
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/internal/runtime/nerdctl.go
- backend/internal/runtime/lima.yaml
- backend/internal/runtime/lima.go
- Makefile
- backend/cmd/calf/main.go
- ui/lib/main.dart
…and wait group - Enhanced the watchdog mechanism in `watchdog_windows.go` to utilize a goroutine and sync.WaitGroup for better management of the parent process monitoring. - Implemented a polling mechanism to check the parent process status, allowing for graceful shutdown when the parent process exits. - Improved error handling and logging for the parent process wait operation.
1 participant
Summary by CodeRabbit