Skip to content

fix: native Termux runit service backend#424

Merged
SuperCoolPencil merged 1 commit intoSurgeDM:mainfrom
AbhiTheModder:fix/service-manager
May 1, 2026
Merged

fix: native Termux runit service backend#424
SuperCoolPencil merged 1 commit intoSurgeDM:mainfrom
AbhiTheModder:fix/service-manager

Conversation

@AbhiTheModder
Copy link
Copy Markdown
Contributor

@AbhiTheModder AbhiTheModder commented May 1, 2026

kardianos/service detects Android as a sysv platform and returns Interactive()==false, causing s.Run() to block indefinitely waiting for service control signals that never come. This made both the TUI and server commands hang on Termux -- even Ctrl+Q couldn't exit the TUI because s.Run() never reached rootCmd.Execute().

So, we platform split using Go build tags.

Fixes regression from 73bca40 (service support PR).

Greptile Summary

This PR fixes a Termux hang by platform-splitting the service backend via Go build tags: on Android, RunService() always calls rootCmd.Execute() directly (bypassing kardianos/service which misdetects Android as sysv and blocks), while non-Android builds retain the existing kardianos/service path. A new native runit/sv backend is added for Termux service management, with correct $PREFIX and SURGE_SV_DIR/SVDIR handling throughout.

Confidence Score: 5/5

Safe to merge — the fix is well-scoped and the only findings are minor style/redundancy issues

All findings are P2. The core fix (build-tag platform split, sv() env filtering, defaultPrefix() usage) is correct. No logic errors or security concerns were found.

cmd/service_ui_termux.go — redundant double stop on uninstall; all new files missing EOF newline

Important Files Changed

Filename Overview
cmd/runservice_std.go New file: platform-split RunService() for non-Android — correctly delegates to kardianos Interactive() check; missing EOF newline
cmd/runservice_termux.go New file: Android RunService() always calls rootCmd.Execute() directly, fixing the hang described in the PR
cmd/service_termux.go New file: full runit/sv service backend for Termux; sv() correctly strips pre-existing SVDIR= before injecting SURGE_SV_DIR, and all paths use defaultPrefix() instead of hardcoded strings
cmd/service_ui_termux.go New file: TUI service integration for Termux; stopTermuxService() is called redundantly (once in ToggleServiceFunc, once inside uninstallTermuxService)
cmd/service_kardianos.go New file: non-Android kardianos/service backend extracted from old service_test.go; goroutine lifecycle and Windows deadlock guard look correct
cmd/service_ui_std.go New file: non-Android TUI service wiring extracted from root.go into a dedicated configureServiceUI function; no issues
cmd/root.go Inline service wiring replaced with configureServiceUI(&m); clean reduction of concerns
cmd/service_test.go Platform-agnostic tests moved out; remaining TestServiceCommandRegistration is now the only cross-platform test
cmd/service_kardianos_test.go Non-Android lifecycle, toggle, and context-cancellation tests; coverage is adequate for the extracted program struct
cmd/service_termux_test.go Android-only tests for env-var precedence in termuxServiceDir(); covers SURGE_SV_DIR override and SVDIR fallback
cmd/service_android_test.go Android smoke test confirming RunService() does not hang on --help; straightforward and correct
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_ui_termux.go:25-31
**`stopTermuxService` called twice on disable**

When the TUI toggle disables the service, `stopTermuxService()` is called explicitly on line 29, and then `uninstallTermuxService()` (line 30) calls it again internally (line 80). The double `sv down surge` is idempotent but redundant — remove the explicit call here since `uninstallTermuxService` already handles it.

```suggestion
	m.ToggleServiceFunc = func(enable bool) error {
		if enable {
			return installTermuxService()
		}
		return uninstallTermuxService()
	}
```

### Issue 2 of 2
cmd/runservice_std.go:23
**Missing newline at end of file**

All nine new files in this PR (`runservice_std.go`, `runservice_termux.go`, `service_android_test.go`, `service_kardianos.go`, `service_kardianos_test.go`, `service_termux.go`, `service_termux_test.go`, `service_ui_std.go`, `service_ui_termux.go`) are missing a trailing newline. POSIX requires text files to end with a newline and many Go tooling checks (e.g., `gofmt`) flag this as a diff noise issue.

Reviews (7): Last reviewed commit: "fix: native Termux runit service backend" | Re-trigger Greptile

Comment thread cmd/service_test.go Outdated
Comment thread cmd/service_test.go Outdated
@AbhiTheModder AbhiTheModder marked this pull request as draft May 1, 2026 05:14
Comment thread cmd/service_test.go Outdated
@AbhiTheModder AbhiTheModder force-pushed the fix/service-manager branch from 8910e19 to 31010eb Compare May 1, 2026 14:05
@AbhiTheModder AbhiTheModder changed the title fix: skip service manager on Android/Termux to prevent freeze fix: native Termux runit service backend May 1, 2026
@AbhiTheModder
Copy link
Copy Markdown
Contributor Author

@SuperCoolPencil could you retrigger greptile ? Thanks

@SuperCoolPencil SuperCoolPencil marked this pull request as ready for review May 1, 2026 14:20
Comment thread cmd/service_termux.go Outdated
Comment thread cmd/service_ui_termux.go Outdated
@AbhiTheModder AbhiTheModder force-pushed the fix/service-manager branch from 6e90915 to 9e2ee8f Compare May 1, 2026 14:38
Comment thread cmd/service_termux.go Outdated
@AbhiTheModder AbhiTheModder force-pushed the fix/service-manager branch from 9e2ee8f to 9e06c44 Compare May 1, 2026 15:03
Comment thread cmd/service_termux.go Outdated
Comment thread cmd/service_ui_termux.go Outdated
Comment thread cmd/service_ui_termux.go
@AbhiTheModder AbhiTheModder force-pushed the fix/service-manager branch from 9e06c44 to 8ec9218 Compare May 1, 2026 15:11
Comment thread cmd/service_termux.go
@AbhiTheModder AbhiTheModder force-pushed the fix/service-manager branch from 8ec9218 to f5c589d Compare May 1, 2026 15:17
@AbhiTheModder
Copy link
Copy Markdown
Contributor Author

@SuperCoolPencil Done from my side, you can review from your side now :)

cc @LukeGTH

@LukeGTH
Copy link
Copy Markdown

LukeGTH commented May 1, 2026

@SuperCoolPencil Done from my side, you can review from your side now :)

cc @LukeGTH

Perfect, if he releases a new version, he just needs to wait; I've set the package to update automatically

@SuperCoolPencil SuperCoolPencil merged commit a78b50d into SurgeDM:main May 1, 2026
8 checks passed
@AbhiTheModder AbhiTheModder deleted the fix/service-manager branch May 1, 2026 21:31
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.

attempt to fix the Termux Surge-cli

3 participants