refactor(cli): extract shared execx.SpawnDetached helper · Entire

refactor(cli): extract shared execx.SpawnDetached helper

4975ded→main·

gtrrz-victor·2d ago·10 files·+87 added/-238 removed

Collapse the two near-identical detached-spawn platform trios (telemetry __send_analytics, trail __refresh_trail_enablement) into a single portable execx.SpawnDetached. execx already owns the sibling detachFromTTY process-attr logic, so both platforms reuse it; the Unix side now uses Setsid (full session detach) instead of Setpgid.

SpawnDetached is a no-op under in-process go test, so unit tests can never accidentally fork the test binary; call sites keep their spawn seams for assertions.

Also drop issue-number references from comments and test messages in favor of self-contained descriptions.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

01KXJQ2497WRN65JJT79EA7NXZView transcript

Changes

10

package execx

import (
    "context"
    "io"
    "os"
    "os/exec"
    "testing"
)

// SpawnDetached re-execs the current executable as a detached, fire-and-forget
// child running args, surviving the parent's exit (new session on Unix,
// CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS on Windows, via detachFromTTY).
// The child runs in dir (os.TempDir() when empty, so the child never holds the
// parent's working directory), inherits the parent's environment, and has its
// stdout/stderr discarded. Best-effort: every error is swallowed — callers
// treat the spawn as advisory background work.
//
// In-process `go test` runs are a no-op: the current executable is the test
// binary, and re-execing it would fork the whole suite. Tests exercise the
// call sites through their spawn seams instead.
func SpawnDetached(dir string, args ...string) {
    if testing.Testing() {
        return
    }
    executable, err := os.Executable()
    if err != nil {
        return
    }

// context.Background(): the child must outlive the parent, so it is never
    // tied to a cancellable context.
    cmd := exec.CommandContext(context.Background(), executable, args...)
    detachFromTTY(cmd)
    cmd.Dir = dir
    if cmd.Dir == "" {
        cmd.Dir = os.TempDir()
    }
    cmd.Env = os.Environ()
    cmd.Stdout = io.Discard
    cmd.Stderr = io.Discard

if err := cmd.Start(); err != nil {
        return
    }
    // Release the process so it can run independently of the parent.
    //nolint:errcheck // best effort — the child continues regardless
    _ = cmd.Process.Release()
}

// TestHandleLifecycleSessionStart_NoSynchronousNetworkForTrailEnablement // guards against #450 (SessionStart hooks stalling Claude Code startup): the // guards against SessionStart hooks stalling agent startup: the // trails-enablement cache refresh must be handed off to a detached subprocess, // never performed inline on the SessionStart hook path. A slow/unreachable API // host previously added up to trailEnablementSessionStartRefreshTimeout (1s) of

// Blackhole https host: accept connections but never complete the TLS // handshake or respond, so an inline dial stalls until a timeout fires // (mirrors the unreachable-host case that motivated #450) rather than // failing fast. // (mirrors the unreachable-host case that motivated the detached refresh) // rather than failing fast. var dialed int32 var lc net.ListenConfig ln, err := lc.Listen(context.Background(), "tcp", "127.0.0.1:0")

// Backstops: SessionStart neither contacted the API host nor blocked. if got := atomic.LoadInt32(&dialed); got != 0 {

t.Fatalf("SessionStart dialed the trails-enablement API synchronously (#450 regression); the refresh must run out of process") } if elapsed > time.Second {

t.Fatalf("handleLifecycleSessionStart took %v; trails-enablement refresh must be detached, not synchronous (#450)", elapsed) } // TestRunTrailEnablementRefresh_BoundedByTimeoutAgainstUnresponsiveHost // verifies the deferred work spawned for #450 still completes (or at least // verifies the deferred refresh work still completes (or at least // gives up) within its own bounded timeout when the API host never // responds — the network work that used to block SessionStart must still // happen, just out of the hook's critical path, and it must not hang forever.

// "detached refresh command must exit 0 even when the API call fails (best-effort cache warming)") } // TestRefreshTrailEnablementCmd_LogsBackgroundFailureToFile guards the #450 // diagnosability fix: the detached __refresh_trail_enablement child runs with // TestRefreshTrailEnablementCmd_LogsBackgroundFailureToFile guards // diagnosability: the detached __refresh_trail_enablement child runs with // stdout/stderr discarded, so a failing background refresh must still leave a // trail in .entire/logs/entire.log instead of vanishing. The command runs in a // repo with no origin remote, so the scope resolves-and-fails locally (no logData, err := os.ReadFile(filepath.Join(root, ".entire", "logs", "entire.log")) require.NoError(t, err) require.Contains(t, string(logData), "trails enablement refresh skipped: scope unresolved", "background refresh failure must be diagnosable in .entire/logs/entire.log (#450)")

// TestTrailRefreshRecentlySpawned_ThrottlesWithinWindow verifies the spawn-side // guard (#450 follow-up): within trailRefreshSpawnThrottle of a recorded spawn, // guard: within trailRefreshSpawnThrottle of a recorded spawn, // further spawns are suppressed; once the window passes a fresh spawn is allowed // and re-recorded. Without this, an unreachable host — which never writes the // cache, so the hourly TTL never starts — would fork a refresh child on every // invocation. // TestSpawnDetachedTrailEnablementRefresh_CollapsesBurst verifies the throttle is // actually wired into the spawn path: a burst of SessionStart-driven attempts for // the same repo forks a single child, not one per hook (#450 follow-up).

func TestSpawnDetachedTrailEnablementRefresh_CollapsesBurst(t *testing.T) { setupStopTestRepo(t) }

// TestSpawnDetachedTrailEnablementRefresh_CollapsesBurst // verifies the throttle is actually wired into the spawn path: a burst of // SessionStart-driven attempts for the same repo forks a single child, // not one per hook.

func TestSpawnDetachedTrailEnablementRefresh_CollapsesBurst(t *testing.T) { setupStopTestRepo(t) }

``

//go:build !unix && !windows

package telemetry

// spawnDetachedAnalytics is a no-op on non-Unix platforms. // Windows support for detached processes would require different syscall flags // (CREATE_NEW_PROCESS_GROUP, DETACHED_PROCESS), but telemetry is best-effort // so we simply skip it on unsupported platforms. func spawnDetachedAnalytics(string) { // No-op: detached subprocess spawning not implemented for this platform }

//go:build unix

package telemetry

import (
    "context"
    "io"
    "os"
    "os/exec"
    "syscall"
)

// spawnDetachedAnalytics spawns a detached subprocess to send analytics.
// On Unix, this uses process group detachment so the subprocess continues
// after the parent exits.
func spawnDetachedAnalytics(payloadJSON string) {
    executable, err := os.Executable()
    if err != nil {
        return
    }

cmd := exec.CommandContext(context.Background(), executable, "__send_analytics", payloadJSON)

// Detach from parent process group so subprocess survives parent exit
    cmd.SysProcAttr = &syscall.SysProcAttr{
        Setpgid: true,
    }

// Don't hold the working directory
    cmd.Dir = "/"

// Inherit environment (may be needed for network config)
    cmd.Env = os.Environ()

// Discard stdout/stderr to prevent output leaking to parent's terminal
    cmd.Stdout = io.Discard
    cmd.Stderr = io.Discard

// Start the process (non-blocking)
    if err := cmd.Start(); err != nil {
        return
    }

// Release the process so it can run independently
    //nolint:errcheck // Best effort - process should continue regardless
    _ = cmd.Process.Release()
}
``