Feature 115/consolidate daemon to cli #116

noe.trevino · 2026-02-21T17:45:39-06:00

noe.trevino commented

2026-02-21 17:45:39 -06:00

Checklist

Read CONTRIBUTING.md
Ran tests (go test ./... -race) locally
Formatted and linted with gofmt -w . and golangci-lint run ./... respectively
Added tests if necessary (significant changes/complex logic)
Updated documentation if applicable
Updated README.md if applicable

Summary

Closes: #115

daemon management is now easily done with just the cli
no need for two binaries now.

good for setting up the package releases

## Checklist - [x] Read `CONTRIBUTING.md` - [x] Ran tests (`go test ./... -race`) locally - [x] Formatted and linted with `gofmt -w .` and `golangci-lint run ./...` respectively - [x] Added tests if necessary (significant changes/complex logic) - [x] Updated documentation if applicable - [x] Updated `README.md` if applicable ## Summary  Closes: #115 daemon management is now easily done with just the cli no need for two binaries now. good for setting up the package releases

claude[bot] (Migrated from github.com) reviewed

2026-02-21 17:49:06 -06:00

internal/cli/daemon/start_test.go

					
				@ -0,0 +1,82 @@

				package daemon

This test does not exercise runDaemon(). It calls os.MkdirAll directly with a hardcoded 0700, so it tests stdlib behaviour, not production code. If the permission value in runDaemon() changed to e.g. 0755, this test would still pass.

The permission assertion (assert.Equal(t, os.FileMode(0700), info.Mode().Perm())) is already present in TestRunDaemon_CreatesSocketAndShutdown (line 42), making this function both redundant and unable to catch regressions in runDaemon().

This test does not exercise `runDaemon()`. It calls `os.MkdirAll` directly with a hardcoded `0700`, so it tests stdlib behaviour, not production code. If the permission value in `runDaemon()` changed to e.g. `0755`, this test would still pass. The permission assertion (`assert.Equal(t, os.FileMode(0700), info.Mode().Perm())`) is already present in `TestRunDaemon_CreatesSocketAndShutdown` (line 42), making this function both redundant and unable to catch regressions in `runDaemon()`.

claude[bot] (Migrated from github.com) reviewed

2026-02-21 17:49:11 -06:00

internal/cli/daemon/setup.go

					
				@ -0,0 +1,187 @@

				package daemon

The exec.LookPath("systemctl") guard block is duplicated verbatim three times: here in installDaemonService, again in removeDaemonService (line 137), and in stopDaemon in stop.go. A shared helper (e.g. requireSystemctl() error) would eliminate the repetition and make future changes (e.g. a better error message) a single-site edit.

The `exec.LookPath("systemctl")` guard block is duplicated verbatim three times: here in `installDaemonService`, again in `removeDaemonService` (line 137), and in `stopDaemon` in `stop.go`. A shared helper (e.g. `requireSystemctl() error`) would eliminate the repetition and make future changes (e.g. a better error message) a single-site edit.

claude[bot] (Migrated from github.com) reviewed

2026-02-21 17:49:15 -06:00

internal/cli/daemon/setup.go

					
				@ -0,0 +1,187 @@

				package daemon

After=network.target is unnecessary for a daemon that communicates exclusively via a Unix domain socket. It adds an ordering dependency on the network stack, which can delay startup on machines where the network target is slow or absent (e.g. offline or air-gapped systems). Remove it or replace it with After=default.target if an ordering anchor is needed.

`After=network.target` is unnecessary for a daemon that communicates exclusively via a Unix domain socket. It adds an ordering dependency on the network stack, which can delay startup on machines where the network target is slow or absent (e.g. offline or air-gapped systems). Remove it or replace it with `After=default.target` if an ordering anchor is needed.

github-actions[bot] commented

2026-02-21 17:56:28 -06:00

(Migrated from github.com)

Informational Warning:

The snapshot tests have been changed.
Please add a screenshot showing what has been changed, and a description of why it was changed.

If you do not provide an explanation and screenshot in 3 days, this PR will be closed.

@TheNoeTrevino

Changed snapshot files (1 files)

internal/testing/snapshots/cli/testdata/e2e/task-show.golden

**Informational Warning:** The snapshot tests have been changed. Please add a screenshot showing what has been changed, and a description of why it was changed. If you do not provide an explanation and screenshot in 3 days, this PR will be closed. @TheNoeTrevino <details> <summary>Changed snapshot files (1 files)</summary> - `internal/testing/snapshots/cli/testdata/e2e/task-show.golden` </details>

👍 1

noe.trevino commented

2026-02-21 18:12:10 -06:00

snapshot tests now have deterministic date setting. digits would vary before

Sign in to join this conversation.

No reviewers