stacklok / toolhive / 26177286507

66%

Build Type

push

github

Committed by web-flow

Commit Message

Inject spawn seam in RunWorkloadDetached to stop orphan test processes (#5346)

* Inject spawn seam in RunWorkloadDetached

DefaultManager.RunWorkloadDetached re-execs os.Executable() with
`start <basename> --foreground`. Under `go test`, os.Executable()
resolves to the test binary, so the unit test for updateSingleWorkload
spawned `workloads.test start test-workload --foreground` as a
detached child via Setsid. Go's testing.Main ignores the positional
args and re-ran the entire test suite, which called
RunWorkloadDetached again — recursive spawn. Each child was orphaned
to launchd/init and survived after the test binary exited, eventually
consuming high CPU on developer machines.

Extract the spawn step into a function-typed field on DefaultManager
(detachedProcessSpawner) with a withDetachedSpawner option, following
the same pattern as withRetryConfig and withRunnerFactory. Production
uses spawnDetached (unchanged behavior); tests inject a no-op spawner
that returns a fake PID.

Closes #5344

* Address review feedback on spawn seam

- MEDIUM: Preserve original WorkloadStatus ordering. Move SetWorkloadStatus(Starting) and the Error rollback into spawnDetached, so failures during pre-exec setup (os.Executable, xdg.DataFile, GetSecretsPassword) return without any status change — matching the pre-refactor behavior.
- MEDIUM: Add TestDefaultManager_RunWorkloadDetached_SpawnerError to cover the new spawner-error path.
- MEDIUM: Add TestDefaultManager_RunWorkloadDetached_SpawnerSuccess to verify the spawner is invoked and its PID is forwarded.
- LOW: Document that ctx is intentionally not propagated to exec.Command, so the child can outlive the parent.
- LOW: Expand detachedProcessSpawner doc to call out the platform-specific detachment that makes orphan children possible.

Coverage Stats

21 of 30 new or added lines in 1 file covered. (70.0%)

14 existing lines in 5 files now uncovered.

64799 of 98933 relevant lines covered (65.5%)

61.1 hits per line