From 6c6d87c62ea07cac8db196d2c2794f9d16daeb52 Mon Sep 17 00:00:00 2001 From: Junjun Zhang Date: Fri, 25 Sep 2026 09:35:10 +0800 Subject: [PATCH] fix(daemon): carry --__daemonized identity marker through exec-restart (#2739) exec-restart rebuilds the argv from scratch, dropping the --__daemonized marker ForkIntoBackground writes. daemonIdentityMatches keys on that marker (or the ggcode[ argv[0] display name); a restarted daemon's live PID therefore looked like an unrelated process reusing the PID - CheckExistingDaemon deleted the PID file and admitted a SECOND daemon on the next 'ggcode daemon -b'. Fix: append --__daemonized to the rebuilt argv. argv[0] display-name rewriting is deliberately NOT replicated: ExecSelf treats args as the flag list AFTER argv[0], so an injected display name would become a stray positional argument and break cobra parsing; Contains-based identity matching needs the flag alone. The daemonized=true branch the flag selects is behaviorally identical for the restart path (the distinguishing parameter is ignored). Tests: flag-combo parse pin (cmd) + identity match/miss table via the same-package cmdline hook (internal/daemon); both suites green. --- cmd/ggcode/daemon.go | 11 ++++++++ cmd/ggcode/zz_issue2739_test.go | 42 ++++++++++++++++++++++++++++ internal/daemon/zz_issue2739_test.go | 37 ++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 cmd/ggcode/zz_issue2739_test.go create mode 100644 internal/daemon/zz_issue2739_test.go diff --git a/cmd/ggcode/daemon.go b/cmd/ggcode/daemon.go index 370582bcf..1e0b680e2 100644 --- a/cmd/ggcode/daemon.go +++ b/cmd/ggcode/daemon.go @@ -1668,10 +1668,21 @@ loop: runfile.Remove(ses.ID) var args []string + // #2739: exec-restart must carry the daemon identity marker, or + // daemonIdentityMatches (which keys on "--__daemonized" / + // "ggcode[") sees the restarted PID as an unrelated process that + // happens to reuse the PID, deletes the PID file, and admits a + // second daemon. ForkIntoBackground writes the same marker + // (internal/daemon/background.go:282). argv[0] display-name + // rewriting is deliberately NOT replicated: ExecSelf treats args + // as the flag list after argv[0], so an injected display name + // would become a stray positional argument and break cobra + // parsing. Contains-based identity matching needs the flag alone. if cfgFile != "" { args = append(args, "--config", cfgFile) } args = append(args, "daemon", "--follow") + args = append(args, "--__daemonized") if ses.ID != "" { args = append(args, "--resume", ses.ID) } diff --git a/cmd/ggcode/zz_issue2739_test.go b/cmd/ggcode/zz_issue2739_test.go new file mode 100644 index 000000000..d86d9d524 --- /dev/null +++ b/cmd/ggcode/zz_issue2739_test.go @@ -0,0 +1,42 @@ +package main + +// #2739 regression (cmd side): the exec-restart argv must carry +// --__daemonized, or daemonIdentityMatches sees the restarted PID as an +// unrelated process (PID-reuse false positive), deletes the PID file, and +// admits a second daemon. This test pins that the daemon command accepts +// the hidden flag combined with the exact restart flags exec-restart +// builds; the identity-match half lives in internal/daemon (same-package +// test hook access). + +import ( + "testing" + + "github.com/spf13/cobra" +) + +func TestIssue2739_DaemonizedFlagParsesWithRestartFlags(t *testing.T) { + // The exact flag combination exec-restart builds: + // [--config X] daemon --follow --__daemonized [--resume ID] [--bypass] ... + root := NewRootCmd() + // The flags live on the daemon subcommand; locate it like a real + // invocation would. + var daemonCmd *cobra.Command + for _, c := range root.Commands() { + if c.Name() == "daemon" { + daemonCmd = c + break + } + } + if daemonCmd == nil { + t.Fatalf("daemon subcommand not found") + } + if err := daemonCmd.ParseFlags([]string{ + "--follow", "--__daemonized", "--bypass", + }); err != nil { + t.Fatalf("exec-restart flag combo must parse: %v", err) + } + f := daemonCmd.Flags() + if v, _ := f.GetBool("__daemonized"); !v { + t.Fatalf("__daemonized must be settable via combined restart argv") + } +} diff --git a/internal/daemon/zz_issue2739_test.go b/internal/daemon/zz_issue2739_test.go new file mode 100644 index 000000000..43598782a --- /dev/null +++ b/internal/daemon/zz_issue2739_test.go @@ -0,0 +1,37 @@ +package daemon + +// #2739 regression (identity side): a daemon restarted via exec-restart +// carries --__daemonized in its rebuilt argv; daemonIdentityMatches must +// recognize it (and must NOT recognize a marker-less cmdline - that is the +// PID-reuse false positive that deletes the PID file and admits a second +// daemon). + +import ( + "strings" + "testing" +) + +func TestIssue2739_IdentityMatchesRestartedCmdline(t *testing.T) { + orig := testProcessCmdline + t.Cleanup(func() { testProcessCmdline = orig }) + + // Simulated /proc//cmdline after exec-restart with the fix. + cmdline := strings.Join([]string{ + "/usr/local/bin/ggcode", "--config", "/home/u/.ggcode/ggcode.yaml", + "daemon", "--follow", "--__daemonized", "--resume", "abc123", + }, "\x00") + testProcessCmdline = func(int) string { return cmdline } + if !daemonIdentityMatches(1234, "/home/u/proj") { + t.Fatalf("restarted daemon cmdline with --__daemonized must match identity") + } + + // Without the marker (the pre-fix bug): identity must NOT match, so a + // live restarted daemon was misjudged as PID reuse. + noMarker := strings.Join([]string{ + "/usr/local/bin/ggcode", "--config", "/x.yaml", "daemon", "--follow", + }, "\x00") + testProcessCmdline = func(int) string { return noMarker } + if daemonIdentityMatches(1234, "/home/u/proj") { + t.Fatalf("cmdline without markers must not match daemon identity") + } +}