From 6f09fc97ae3270bb2a6ad40b9e0df2c9949764cf Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Fri, 18 Sep 2026 08:24:27 -0700 Subject: [PATCH] fix(cli): stop killing the background listen child with --pid-file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `relayfile listen --background` never worked. The parent appended `--pid-file ` to the child's argv, but `runListen` never registered that flag, so the child exited at flag parsing: $ relayfile listen --background Listen started in background. Logs: ~/.relayfile/listen.log $ cat ~/.relayfile/listen.log error: flag provided but not defined: -pid-file The parent had already printed success and released the process, so the failure was invisible unless the user opened the log. `runListen` parses with flag.ContinueOnError and discards the flagset's output, which is why nothing surfaced on a terminal. Nothing ever read the listen pid file — there is no `listen stop` or `listen status`, and `listenPIDFile` had no other caller — so the flag is dropped rather than registered, and the dead helper goes with it. Child argv construction moves into `backgroundListenChildArgs` so the invariant is testable: the new test asserts the argv `--background` actually re-invokes parses under the real flagset, instead of pinning a literal argv that would drift. Reintroducing `--pid-file` fails it. Co-Authored-By: Claude Opus 5 (1M context) Session-Id: d458bd97-53d8-4f02-be9c-48b67b93c916 --- cmd/relayfile-cli/background_test.go | 36 +++++++++++++++++++++++ cmd/relayfile-cli/main.go | 43 ++++++++++++++++------------ 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/cmd/relayfile-cli/background_test.go b/cmd/relayfile-cli/background_test.go index 94f431ca..8950326e 100644 --- a/cmd/relayfile-cli/background_test.go +++ b/cmd/relayfile-cli/background_test.go @@ -5,10 +5,12 @@ package main import ( "bytes" "errors" + "io" "os" "os/exec" "path/filepath" "reflect" + "slices" "strconv" "strings" "syscall" @@ -841,3 +843,37 @@ func TestA14LogsCommandTailsBackgroundLog(t *testing.T) { } } } + +// TestBackgroundListenChildArgsOnlyUseRegisteredFlags pins the contract that +// broke `relayfile listen --background` entirely: the parent appended +// `--pid-file`, which `runListen` never registered, so the child died at flag +// parsing while the parent reported "Listen started in background". +// +// `runListen` parses with flag.ContinueOnError and a discarded output sink, so +// an unknown flag produces no diagnostic anywhere the user looks. Asserting on +// the parse outcome — rather than on a hardcoded argv — keeps any future flag +// added to the child honest. +func TestBackgroundListenChildArgsOnlyUseRegisteredFlags(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + clearRelayfileEnv(t) + + args := backgroundListenChildArgs([]string{"--provider", "linear", "--background"}) + + if slices.Contains(args, "--background") { + t.Fatalf("child argv still carries --background: %v", args) + } + if !slices.Contains(args, "--daemonized") { + t.Fatalf("child argv is missing --daemonized: %v", args) + } + + // runListen fails on credentials in a bare HOME, which is fine: the point + // is that it gets past flag parsing at all. Before the fix it returned + // "flag provided but not defined: -pid-file" and never reached the work. + err := runListen(args, io.Discard) + if err == nil { + return + } + if strings.Contains(err.Error(), "flag provided but not defined") { + t.Fatalf("child argv %v carries a flag runListen does not register: %v", args, err) + } +} diff --git a/cmd/relayfile-cli/main.go b/cmd/relayfile-cli/main.go index 96b16338..7a8e6de6 100644 --- a/cmd/relayfile-cli/main.go +++ b/cmd/relayfile-cli/main.go @@ -8652,9 +8652,7 @@ func runListen(args []string, stdout io.Writer) error { } if *background && !*daemonized { - logFile := listenLogFile() - pidFile := listenPIDFile() - return spawnBackgroundListenProcess(args, pidFile, logFile) + return spawnBackgroundListenProcess(args, listenLogFile()) } if *daemonized { if err := rotateLogFile(listenLogFile()); err != nil { @@ -8881,23 +8879,22 @@ func runListenSession(rootCtx context.Context, cfg listenSessionConfig) (bool, e } } -func listenPIDFile() string { - return filepath.Join(configDir(), "listen.pid") -} - func listenLogFile() string { return filepath.Join(configDir(), "listen.log") } -func spawnBackgroundListenProcess(originalArgs []string, pidFile, logFile string) error { - if err := rotateLogFile(logFile); err != nil { - return err - } - executable, err := os.Executable() - if err != nil { - return err - } - filtered := make([]string, 0, len(originalArgs)) +// backgroundListenChildArgs builds the argv `listen --background` re-invokes +// itself with: the caller's own flags minus `--background`, plus the internal +// `--daemonized` marker. +// +// Every flag here must be one `runListen` registers. Its flagset is +// ContinueOnError with output discarded, so an unrecognised flag makes the +// child exit immediately with nothing on a terminal, while the parent has +// already printed "Listen started in background". `--pid-file` was passed for +// exactly that reason and broke every background listen; nothing read the +// listen pid file, so it was dropped rather than registered. +func backgroundListenChildArgs(originalArgs []string) []string { + filtered := make([]string, 0, len(originalArgs)+1) for _, arg := range originalArgs { if arg == "--background" || arg == "-background" || strings.HasPrefix(arg, "--background=") || strings.HasPrefix(arg, "-background=") { @@ -8905,8 +8902,18 @@ func spawnBackgroundListenProcess(originalArgs []string, pidFile, logFile string } filtered = append(filtered, arg) } - childArgs := append([]string{"listen"}, filtered...) - childArgs = append(childArgs, "--daemonized", "--pid-file", pidFile) + return append(filtered, "--daemonized") +} + +func spawnBackgroundListenProcess(originalArgs []string, logFile string) error { + if err := rotateLogFile(logFile); err != nil { + return err + } + executable, err := os.Executable() + if err != nil { + return err + } + childArgs := append([]string{"listen"}, backgroundListenChildArgs(originalArgs)...) logHandle, err := os.OpenFile(logFile, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) if err != nil { return err