Skip to content

feat: official auto-start service support#422

Merged
SuperCoolPencil merged 19 commits intomainfrom
feat/official-service-support
Apr 30, 2026
Merged

feat: official auto-start service support#422
SuperCoolPencil merged 19 commits intomainfrom
feat/official-service-support

Conversation

@SuperCoolPencil
Copy link
Copy Markdown
Member

@SuperCoolPencil SuperCoolPencil commented Apr 30, 2026

This PR adds a built-in service command to manage Surge as a system service (daemon) using the kardianos/service library.

Key Changes:

  • Added surge service command with install, uninstall, start, stop, and status subcommands.
  • Integrated service support into root.go with minimal changes to existing logic.
  • Cross-platform support (Linux, Windows, macOS) via kardianos/service.

Why this matters:

  • Easier automated provisioning and reproducible deployments.
  • Official way to handle auto-start without third-party wrappers or manual cron/Task Scheduler setup.

Note on minimal changes:

The implementation wraps the existing rootCmd.Execute() logic, ensuring that when run as a service, Surge behaves exactly like surge server start but integrates correctly with the OS service manager (especially important for Windows SCM).

Greptile Summary

This PR adds a built-in surge service command backed by kardianos/service for cross-platform daemon management, and wires an auto_start toggle into the TUI settings so users can install/uninstall the OS service without leaving the app. The implementation addresses all the lifecycle, shutdown, and state-consistency concerns from earlier review rounds.

Confidence Score: 5/5

Safe to merge; only P2 findings remain — a misleading test and a minor explicit-value bypass in the settings path.

All previously flagged P0/P1 issues (deadlock, os.Exit lifecycle bypass, silent status error, ToggleServiceFunc nil no-op, unconditional uninstall on reset) have been addressed. The two remaining findings are P2: a test that doesn't actually exercise the context-cancellation path, and an edge-case gap in the explicit-value bool-setting branch. Neither affects production behaviour in the current codebase.

cmd/service_test.go — TestProgramContextCancellation doesn't exercise the cancellation path it claims to test.

Important Files Changed

Filename Overview
cmd/service.go New file implementing the service daemon lifecycle (Start/Stop via context cancellation) and CLI subcommands; previous threading issues (deadlock, os.Exit, StatusNotInstalled) are all addressed.
cmd/service_test.go New test file; TestProgramContextCancellation exits immediately via --help so context cancellation is never actually exercised, making the test name misleading and the shutdown path untested.
cmd/root.go Integrates service status into TUI startup; previously flagged issues (s.Start() conflict, silent Status() error) are addressed; ToggleServiceFunc only installs, not starts.
internal/tui/view_settings.go auto_start toggle and reset now properly call ToggleServiceFunc with error propagation; explicit-value path for auto_start bypasses ToggleServiceFunc. Layout refactor for error line height looks correct.
internal/tui/update_settings.go Bool toggle and reset now handle errors from setSettingValue/resetSettingToDefault and surface them via settingsError instead of silently discarding.
internal/config/settings.go Added AutoStart field with correct JSON key, UI label, and description; no restart tag needed since the TUI toggle handles service install/uninstall directly.
cmd/server.go Added cmd.Context().Done() case to both select blocks so service context cancellation cleanly shuts down the server.
internal/tui/settings_reset_test.go Updated to handle the new error return from resetSettingToDefault; auto_start reset works correctly with nil ToggleServiceFunc in test context.
go.mod github.com/kardianos/service added correctly as a direct dependency without the // indirect comment.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
cmd/service_test.go:96-113
**`TestProgramContextCancellation` doesn't exercise context cancellation**

`rootCmd.SetArgs([]string{"--help"})` makes `rootCmd.ExecuteContext(ctx)` return almost immediately (cobra prints help and returns `nil`). By the time `waitStop` calls `p.Stop(s)`, the goroutine has already exited and `p.exit` is already closed. The `cancel` function is captured but the shutdown path through context cancellation is never actually exercised; the test passes vacuously.

To meaningfully test this, the goroutine needs to be blocked (e.g., on a channel or a long-running command) so that `p.cancel()` inside `Stop` is what causes it to unblock and return.

### Issue 2 of 2
internal/tui/view_settings.go:667-685
**`ToggleServiceFunc` bypassed when `auto_start` is set with an explicit value**

The `auto_start` special-casing only runs when `value == ""` (the toggle path). When `value != ""`, execution falls through to the generic `strconv.ParseBool` branch which sets the field directly without calling `ToggleServiceFunc`. Any caller that passes an explicit `"true"` or `"false"` — including a future API handler or test helper — will silently diverge the in-memory setting from the actual OS service state. Consider hoisting the `auto_start` guard to cover both branches:

```go
case reflect.Bool:
    if key == "auto_start" {
        newVal := !targetField.Bool()
        if value != "" {
            newVal, _ = strconv.ParseBool(value)
        }
        if m.ToggleServiceFunc == nil {
            return fmt.Errorf("service management is not available on this platform")
        }
        if err := m.ToggleServiceFunc(newVal); err != nil {
            return fmt.Errorf("failed to update service: %w", err)
        }
        targetField.SetBool(newVal)
        return nil
    }
    if value == "" {
        targetField.SetBool(!targetField.Bool())
    } else {
        b, _ := strconv.ParseBool(value)
        targetField.SetBool(b)
    }
```

Reviews (9): Last reviewed commit: "test: mock command arguments in service ..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Binary Size Analysis

⚠️ Size Increased

Version Human Readable Raw Bytes
Main 19.61 MB 20562212
PR 19.76 MB 20717860
Difference 152.00 KB 155648

Comment thread cmd/service.go
Comment thread cmd/service.go
Comment thread go.mod Outdated
Comment thread cmd/service.go Outdated
Comment thread cmd/service.go
Comment thread internal/tui/view_settings.go
Comment thread internal/tui/view_settings.go
Comment thread cmd/service.go Outdated
Comment thread internal/tui/view_settings.go
Comment thread cmd/root.go
Comment thread cmd/root.go Outdated
Comment thread internal/tui/view_settings.go
Comment thread cmd/service.go
Comment thread cmd/service_test.go
@SuperCoolPencil SuperCoolPencil merged commit 73bca40 into main Apr 30, 2026
16 checks passed
@SuperCoolPencil SuperCoolPencil deleted the feat/official-service-support branch April 30, 2026 18:48
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