fix: prevent orphaned desktop backends from holding locks (#2802)
This commit is contained in:
Carlos Florêncio
@@ -14,10 +14,15 @@ var ErrConflict = errors.New("runtime state is already owned")
|
||||
// ConflictError identifies the target that prevented startup.
|
||||
type ConflictError struct {
|
||||
Target Target
|
||||
Owner *OwnerRecord
|
||||
}
|
||||
|
||||
func (e *ConflictError) Error() string {
|
||||
return fmt.Sprintf("%s target %q is already owned by another backend", e.Target.Kind, e.Target.ResourcePath)
|
||||
message := fmt.Sprintf("%s target %q is already owned by another backend", e.Target.Kind, e.Target.ResourcePath)
|
||||
if e.Owner != nil {
|
||||
message += " (" + e.Owner.String() + ")"
|
||||
}
|
||||
return message
|
||||
}
|
||||
|
||||
func (e *ConflictError) Unwrap() error { return ErrConflict }
|
||||
@@ -55,10 +60,11 @@ func Acquire(targets []Target) (*Owner, error) {
|
||||
_ = file.Close()
|
||||
_ = owner.Close()
|
||||
if errors.Is(err, ErrConflict) {
|
||||
return nil, &ConflictError{Target: target}
|
||||
return nil, &ConflictError{Target: target, Owner: readOwner(target.LockPath)}
|
||||
}
|
||||
return nil, fmt.Errorf("acquire %s ownership lock %q: %w", target.Kind, target.LockPath, err)
|
||||
}
|
||||
_ = writeOwnerFn(file)
|
||||
owner.held = append(owner.held, heldLock{target: target, file: file})
|
||||
}
|
||||
return owner, nil
|
||||
@@ -68,7 +74,7 @@ func openLockFile(path string) (*os.File, error) {
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
|
||||
return openLockFilePlatform(path)
|
||||
}
|
||||
|
||||
// Close releases every held OS lock in reverse acquisition order. The first
|
||||
|
||||
@@ -2,8 +2,11 @@ package ownershiplock
|
||||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -86,6 +89,32 @@ func TestOwnershipLockReleasesAfterForcedProcessExit(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnershipLockRejectsSymlinkedLockPath(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
target := filepath.Join(dir, "target")
|
||||
link := filepath.Join(dir, "lock")
|
||||
want := []byte("must remain unchanged")
|
||||
if err := os.WriteFile(target, want, 0o600); err != nil {
|
||||
t.Fatalf("write target: %v", err)
|
||||
}
|
||||
if err := os.Symlink(target, link); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
|
||||
file, err := openLockFile(link)
|
||||
if err == nil {
|
||||
_ = file.Close()
|
||||
t.Fatal("openLockFile succeeded for symlinked lock path")
|
||||
}
|
||||
got, err := os.ReadFile(target)
|
||||
if err != nil {
|
||||
t.Fatalf("read target: %v", err)
|
||||
}
|
||||
if !bytes.Equal(got, want) {
|
||||
t.Fatalf("symlink target changed to %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnershipLockAllowsIndependentHomes(t *testing.T) {
|
||||
firstHome := t.TempDir()
|
||||
firstTargets, err := Targets(firstHome, "sqlite", "")
|
||||
@@ -223,4 +252,126 @@ func TestOwnershipLockConflictErrorIncludesTarget(t *testing.T) {
|
||||
if conflict.Target.ResourcePath != targets[0].ResourcePath {
|
||||
t.Fatalf("conflict target = %q, want canonical home path %q", conflict.Target.ResourcePath, targets[0].ResourcePath)
|
||||
}
|
||||
if conflict.Owner == nil {
|
||||
t.Fatal("conflict owner metadata is nil")
|
||||
}
|
||||
if conflict.Owner.PID != int64(os.Getpid()) {
|
||||
t.Fatalf("conflict owner PID = %d, want %d", conflict.Owner.PID, os.Getpid())
|
||||
}
|
||||
if !strings.Contains(conflict.Error(), fmt.Sprintf("pid %d", os.Getpid())) {
|
||||
t.Fatalf("conflict error = %q, want owner PID", conflict.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnershipLockConflictFallsBackWithoutUsableOwner(t *testing.T) {
|
||||
oldLiveness := ownerLivenessFn
|
||||
ownerLivenessFn = func(int64) (bool, bool) { return false, true }
|
||||
t.Cleanup(func() { ownerLivenessFn = oldLiveness })
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
data []byte
|
||||
}{
|
||||
{name: "empty", data: ownerMetadataFixture("")},
|
||||
{name: "invalid", data: ownerMetadataFixture("not json")},
|
||||
{name: "dead", data: ownerMetadataBytes(mustOwnerJSON(t, OwnerRecord{PID: 987654321, Executable: "dead-kandev", StartedAt: "2026-08-19T09:14:35Z"}))},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
targets, err := Targets(home, "sqlite", "")
|
||||
if err != nil {
|
||||
t.Fatalf("Targets: %v", err)
|
||||
}
|
||||
owner, err := Acquire(targets)
|
||||
if err != nil {
|
||||
t.Fatalf("Acquire primary owner: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = owner.Close() })
|
||||
if err := writeOwnerMetadataFixture(owner.held[0].file, test.data); err != nil {
|
||||
t.Fatalf("write conflict metadata: %v", err)
|
||||
}
|
||||
|
||||
_, err = Acquire(targets)
|
||||
var conflict *ConflictError
|
||||
if !errors.As(err, &conflict) {
|
||||
t.Fatalf("second Acquire error = %v, want ConflictError", err)
|
||||
}
|
||||
want := fmt.Sprintf("%s target %q is already owned by another backend", TargetHome, targets[0].ResourcePath)
|
||||
if conflict.Error() != want {
|
||||
t.Fatalf("conflict error = %q, want %q", conflict.Error(), want)
|
||||
}
|
||||
if conflict.Owner != nil {
|
||||
t.Fatalf("conflict owner = %+v, want nil", conflict.Owner)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func writeOwnerMetadataFixture(file *os.File, data []byte) error {
|
||||
if err := file.Truncate(0); err != nil {
|
||||
return err
|
||||
}
|
||||
n, err := file.WriteAt(data, 0)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if n != len(data) {
|
||||
return io.ErrShortWrite
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestOwnershipLockConflictKeepsUnknownOwner(t *testing.T) {
|
||||
oldLiveness := ownerLivenessFn
|
||||
ownerLivenessFn = func(int64) (bool, bool) { return false, false }
|
||||
t.Cleanup(func() { ownerLivenessFn = oldLiveness })
|
||||
|
||||
home := t.TempDir()
|
||||
targets, err := Targets(home, "sqlite", "")
|
||||
if err != nil {
|
||||
t.Fatalf("Targets: %v", err)
|
||||
}
|
||||
owner, err := Acquire(targets)
|
||||
if err != nil {
|
||||
t.Fatalf("Acquire primary owner: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = owner.Close() })
|
||||
|
||||
_, err = Acquire(targets)
|
||||
var conflict *ConflictError
|
||||
if !errors.As(err, &conflict) {
|
||||
t.Fatalf("second Acquire error = %v, want ConflictError", err)
|
||||
}
|
||||
if conflict.Owner == nil {
|
||||
t.Fatal("unknown liveness dropped owner metadata")
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnershipLockMetadataWriteFailureDoesNotBlockAcquire(t *testing.T) {
|
||||
oldWriteOwner := writeOwnerFn
|
||||
writeOwnerFn = func(*os.File) error { return errors.New("metadata unavailable") }
|
||||
t.Cleanup(func() { writeOwnerFn = oldWriteOwner })
|
||||
|
||||
home := t.TempDir()
|
||||
targets, err := Targets(home, "sqlite", "")
|
||||
if err != nil {
|
||||
t.Fatalf("Targets: %v", err)
|
||||
}
|
||||
owner, err := Acquire(targets)
|
||||
if err != nil {
|
||||
t.Fatalf("Acquire with metadata failure: %v", err)
|
||||
}
|
||||
if err := owner.Close(); err != nil {
|
||||
t.Fatalf("Close: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func mustOwnerJSON(t *testing.T, owner OwnerRecord) []byte {
|
||||
t.Helper()
|
||||
data, err := json.Marshal(owner)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal owner: %v", err)
|
||||
}
|
||||
return data
|
||||
}
|
||||
|
||||
@@ -4,11 +4,34 @@ package ownershiplock
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
|
||||
"golang.org/x/sys/unix"
|
||||
)
|
||||
|
||||
func openLockFilePlatform(path string) (*os.File, error) {
|
||||
fd, err := unix.Open(path, unix.O_RDWR|unix.O_CREAT|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0o600)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
file := os.NewFile(uintptr(fd), path)
|
||||
if file == nil {
|
||||
_ = unix.Close(fd)
|
||||
return nil, fmt.Errorf("create ownership lock file handle for %q", path)
|
||||
}
|
||||
info, err := file.Stat()
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return nil, err
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
_ = file.Close()
|
||||
return nil, fmt.Errorf("ownership lock path %q is not a regular file", path)
|
||||
}
|
||||
return file, nil
|
||||
}
|
||||
|
||||
func lockFile(file *os.File) error {
|
||||
err := unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB)
|
||||
if err == nil {
|
||||
|
||||
@@ -4,11 +4,51 @@ package ownershiplock
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
|
||||
"golang.org/x/sys/windows"
|
||||
)
|
||||
|
||||
func openLockFilePlatform(path string) (*os.File, error) {
|
||||
handle, err := windows.CreateFile(
|
||||
windows.StringToUTF16Ptr(path),
|
||||
windows.GENERIC_READ|windows.GENERIC_WRITE,
|
||||
windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE,
|
||||
nil,
|
||||
windows.OPEN_ALWAYS,
|
||||
windows.FILE_ATTRIBUTE_NORMAL|windows.FILE_FLAG_OPEN_REPARSE_POINT,
|
||||
0,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
file := os.NewFile(uintptr(handle), path)
|
||||
if file == nil {
|
||||
_ = windows.CloseHandle(handle)
|
||||
return nil, fmt.Errorf("create ownership lock file handle for %q", path)
|
||||
}
|
||||
fileType, err := windows.GetFileType(handle)
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return nil, err
|
||||
}
|
||||
var info windows.ByHandleFileInformation
|
||||
if err := windows.GetFileInformationByHandle(handle, &info); err != nil {
|
||||
_ = file.Close()
|
||||
return nil, err
|
||||
}
|
||||
if info.FileAttributes&windows.FILE_ATTRIBUTE_REPARSE_POINT != 0 {
|
||||
_ = file.Close()
|
||||
return nil, fmt.Errorf("ownership lock path %q is a reparse point", path)
|
||||
}
|
||||
if info.FileAttributes&windows.FILE_ATTRIBUTE_DIRECTORY != 0 || fileType != windows.FILE_TYPE_DISK {
|
||||
_ = file.Close()
|
||||
return nil, fmt.Errorf("ownership lock path %q is not a regular file", path)
|
||||
}
|
||||
return file, nil
|
||||
}
|
||||
|
||||
func lockFile(file *os.File) error {
|
||||
err := windows.LockFileEx(
|
||||
windows.Handle(file.Fd()),
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
package ownershiplock
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/proclive"
|
||||
)
|
||||
|
||||
// OwnerRecord is advisory diagnostic metadata about the process that acquired
|
||||
// a lock. It is never consulted to decide ownership, staleness, or takeover.
|
||||
type OwnerRecord struct {
|
||||
PID int64 `json:"pid"`
|
||||
Executable string `json:"executable"`
|
||||
StartedAt string `json:"started_at"`
|
||||
}
|
||||
|
||||
var (
|
||||
writeOwnerFn = writeOwner
|
||||
ownerLivenessFn = proclive.Alive
|
||||
)
|
||||
|
||||
const (
|
||||
// Windows locks the first byte of the file. Keep metadata after that byte
|
||||
// so a conflicting process can read it through a separate file handle.
|
||||
ownerMetadataOffset int64 = 1
|
||||
maxOwnerMetadataLength int64 = 4096
|
||||
)
|
||||
|
||||
func writeOwner(file *os.File) error {
|
||||
executable, _ := os.Executable()
|
||||
record := OwnerRecord{
|
||||
PID: int64(os.Getpid()),
|
||||
Executable: executable,
|
||||
StartedAt: time.Now().UTC().Format(time.RFC3339Nano),
|
||||
}
|
||||
data, err := json.Marshal(record)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := file.Truncate(0); err != nil {
|
||||
return err
|
||||
}
|
||||
n, err := file.WriteAt(data, ownerMetadataOffset)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if n != len(data) {
|
||||
return io.ErrShortWrite
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func readOwner(path string) *OwnerRecord {
|
||||
file, err := os.Open(path)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
defer func() { _ = file.Close() }()
|
||||
info, err := file.Stat()
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
metadataLength := info.Size() - ownerMetadataOffset
|
||||
if metadataLength <= 0 || metadataLength > maxOwnerMetadataLength {
|
||||
return nil
|
||||
}
|
||||
data := make([]byte, int(metadataLength))
|
||||
if _, err := file.ReadAt(data, ownerMetadataOffset); err != nil {
|
||||
return nil
|
||||
}
|
||||
var record OwnerRecord
|
||||
if err := json.Unmarshal(data, &record); err != nil || record.PID <= 0 {
|
||||
return nil
|
||||
}
|
||||
alive, known := ownerLivenessFn(record.PID)
|
||||
if known && !alive {
|
||||
return nil
|
||||
}
|
||||
return &record
|
||||
}
|
||||
|
||||
func (r *OwnerRecord) String() string {
|
||||
if r == nil {
|
||||
return ""
|
||||
}
|
||||
detail := fmt.Sprintf("pid %d", r.PID)
|
||||
if r.Executable != "" {
|
||||
detail += ", " + r.Executable
|
||||
}
|
||||
if r.StartedAt != "" {
|
||||
detail += ", started " + r.StartedAt
|
||||
}
|
||||
return detail
|
||||
}
|
||||
@@ -0,0 +1,118 @@
|
||||
package ownershiplock
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestWriteOwnerRoundTrips(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "lock")
|
||||
file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("open lock file: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = file.Close() })
|
||||
|
||||
if err := writeOwner(file); err != nil {
|
||||
t.Fatalf("writeOwner: %v", err)
|
||||
}
|
||||
owner := readOwner(path)
|
||||
if owner == nil {
|
||||
t.Fatal("readOwner returned nil")
|
||||
return
|
||||
}
|
||||
if owner.PID != int64(os.Getpid()) {
|
||||
t.Fatalf("owner PID = %d, want %d", owner.PID, os.Getpid())
|
||||
}
|
||||
if owner.Executable == "" {
|
||||
t.Fatal("owner executable is empty")
|
||||
}
|
||||
if owner.StartedAt == "" {
|
||||
t.Fatal("owner start time is empty")
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadOwnerWhileLockIsHeld(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "lock")
|
||||
file, err := openLockFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("open lock file: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
_ = unlockFile(file)
|
||||
_ = file.Close()
|
||||
})
|
||||
if err := lockFile(file); err != nil {
|
||||
t.Fatalf("lockFile: %v", err)
|
||||
}
|
||||
if err := writeOwner(file); err != nil {
|
||||
t.Fatalf("writeOwner: %v", err)
|
||||
}
|
||||
|
||||
owner := readOwner(path)
|
||||
if owner == nil {
|
||||
t.Fatal("readOwner returned nil while lock was held")
|
||||
return
|
||||
}
|
||||
if owner.PID != int64(os.Getpid()) {
|
||||
t.Fatalf("owner PID = %d, want %d", owner.PID, os.Getpid())
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteOwnerTruncatesPreviousRecord(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "lock")
|
||||
file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
|
||||
if err != nil {
|
||||
t.Fatalf("open lock file: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = file.Close() })
|
||||
if _, err := file.WriteString(`{"pid":123456789,"executable":"` + string(make([]byte, 4096)) + `"}`); err != nil {
|
||||
t.Fatalf("write previous record: %v", err)
|
||||
}
|
||||
|
||||
if err := writeOwner(file); err != nil {
|
||||
t.Fatalf("writeOwner: %v", err)
|
||||
}
|
||||
owner := readOwner(path)
|
||||
if owner == nil {
|
||||
t.Fatal("readOwner returned nil after truncating previous metadata")
|
||||
return
|
||||
}
|
||||
if owner.PID != int64(os.Getpid()) {
|
||||
t.Fatalf("owner PID = %d, want %d", owner.PID, os.Getpid())
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadOwnerRejectsInvalidMetadata(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
data string
|
||||
}{
|
||||
{name: "missing", data: ""},
|
||||
{name: "invalid json", data: "not json"},
|
||||
{name: "missing pid", data: `{"executable":"kandev"}`},
|
||||
{name: "non-positive pid", data: `{"pid":0}`},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "lock")
|
||||
if test.name != "missing" {
|
||||
if err := os.WriteFile(path, ownerMetadataFixture(test.data), 0o600); err != nil {
|
||||
t.Fatalf("write metadata: %v", err)
|
||||
}
|
||||
}
|
||||
if owner := readOwner(path); owner != nil {
|
||||
t.Fatalf("readOwner = %+v, want nil", owner)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func ownerMetadataFixture(data string) []byte {
|
||||
return ownerMetadataBytes([]byte(data))
|
||||
}
|
||||
|
||||
func ownerMetadataBytes(data []byte) []byte {
|
||||
return append([]byte{0}, data...)
|
||||
}
|
||||
@@ -0,0 +1,5 @@
|
||||
// Package proclive provides platform-specific process liveness probes.
|
||||
//
|
||||
// A false known value means the platform cannot determine liveness. Callers
|
||||
// must treat that result as "do not act".
|
||||
package proclive
|
||||
+4
-2
@@ -1,10 +1,12 @@
|
||||
//go:build !windows
|
||||
|
||||
package tempartifacts
|
||||
package proclive
|
||||
|
||||
import "syscall"
|
||||
|
||||
func processAlive(pid int64) (alive, known bool) {
|
||||
// Alive reports whether pid is alive. Unix signal-zero checks distinguish a
|
||||
// dead process from a process that exists but cannot be signaled by the caller.
|
||||
func Alive(pid int64) (alive, known bool) {
|
||||
if pid <= 0 {
|
||||
return false, true
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
//go:build !windows
|
||||
|
||||
package proclive
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestAliveReportsCurrentProcess(t *testing.T) {
|
||||
alive, known := Alive(int64(os.Getpid()))
|
||||
if !alive || !known {
|
||||
t.Fatalf("Alive(self) = (%t, %t), want (true, true)", alive, known)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAliveReportsReapedProcessAsDead(t *testing.T) {
|
||||
child := exec.Command("sh", "-c", "exit 0")
|
||||
if err := child.Start(); err != nil {
|
||||
t.Fatalf("start child: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
if child.ProcessState == nil {
|
||||
_ = child.Process.Kill()
|
||||
_ = child.Wait()
|
||||
}
|
||||
})
|
||||
if err := child.Wait(); err != nil {
|
||||
t.Fatalf("wait child: %v", err)
|
||||
}
|
||||
|
||||
alive, known := Alive(int64(child.Process.Pid))
|
||||
if alive || !known {
|
||||
t.Fatalf("Alive(reaped child) = (%t, %t), want (false, true)", alive, known)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAliveReportsNonPositivePIDAsDeadAndKnown(t *testing.T) {
|
||||
for _, pid := range []int64{0, -1} {
|
||||
alive, known := Alive(pid)
|
||||
if alive || !known {
|
||||
t.Errorf("Alive(%d) = (%t, %t), want (false, true)", pid, alive, known)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,8 @@
|
||||
//go:build windows
|
||||
|
||||
package proclive
|
||||
|
||||
// Alive cannot portably determine process liveness on Windows.
|
||||
func Alive(int64) (alive, known bool) {
|
||||
return false, false
|
||||
}
|
||||
@@ -0,0 +1,15 @@
|
||||
//go:build windows
|
||||
|
||||
package proclive
|
||||
|
||||
import (
|
||||
"os"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestAliveReportsUnknownOnWindows(t *testing.T) {
|
||||
alive, known := Alive(int64(os.Getpid()))
|
||||
if alive || known {
|
||||
t.Fatalf("Alive(self) = (%t, %t), want (false, false)", alive, known)
|
||||
}
|
||||
}
|
||||
@@ -10,6 +10,7 @@ const (
|
||||
defaultBackendPort = 38429
|
||||
defaultWebPort = 37429
|
||||
defaultAgentctlPort = 39429
|
||||
parentPIDEnv = "KANDEV_LAUNCHER_PARENT_PID"
|
||||
|
||||
healthTimeoutReleaseMS = 45000
|
||||
healthTimeoutDevMS = 600000
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/proclive"
|
||||
)
|
||||
|
||||
const parentWatchPollInterval = 2 * time.Second
|
||||
|
||||
type parentWatchdog struct {
|
||||
parentPID int64
|
||||
interval time.Duration
|
||||
probe func(int64) (alive, known bool)
|
||||
shutdown func(string)
|
||||
exit func(int)
|
||||
|
||||
mu sync.Mutex
|
||||
started bool
|
||||
stopped bool
|
||||
stopCh chan struct{}
|
||||
wg sync.WaitGroup
|
||||
}
|
||||
|
||||
func newParentWatchdog(parentPID int64, shutdown func(string), exit func(int)) *parentWatchdog {
|
||||
return &parentWatchdog{
|
||||
parentPID: parentPID,
|
||||
interval: parentWatchPollInterval,
|
||||
probe: proclive.Alive,
|
||||
shutdown: shutdown,
|
||||
exit: exit,
|
||||
}
|
||||
}
|
||||
|
||||
func newParentWatchdogFromEnv(shutdown func(string), exit func(int)) *parentWatchdog {
|
||||
return newParentWatchdog(parentPIDFromEnv(), shutdown, exit)
|
||||
}
|
||||
|
||||
func parentPIDFromEnv() int64 {
|
||||
raw := strings.TrimSpace(os.Getenv(parentPIDEnv))
|
||||
pid, err := strconv.ParseInt(raw, 10, 64)
|
||||
if err != nil || pid <= 0 {
|
||||
return 0
|
||||
}
|
||||
return pid
|
||||
}
|
||||
|
||||
func (w *parentWatchdog) start() {
|
||||
w.mu.Lock()
|
||||
if w.parentPID <= 0 || w.started || w.stopped {
|
||||
w.mu.Unlock()
|
||||
return
|
||||
}
|
||||
w.started = true
|
||||
w.stopCh = make(chan struct{})
|
||||
w.wg.Add(1)
|
||||
w.mu.Unlock()
|
||||
|
||||
go w.run()
|
||||
}
|
||||
|
||||
func (w *parentWatchdog) stop() {
|
||||
w.mu.Lock()
|
||||
if w.stopped {
|
||||
w.mu.Unlock()
|
||||
return
|
||||
}
|
||||
w.stopped = true
|
||||
started := w.started
|
||||
if started {
|
||||
close(w.stopCh)
|
||||
}
|
||||
w.mu.Unlock()
|
||||
if started {
|
||||
w.wg.Wait()
|
||||
}
|
||||
}
|
||||
|
||||
func (w *parentWatchdog) run() {
|
||||
defer w.wg.Done()
|
||||
ticker := time.NewTicker(w.interval)
|
||||
defer ticker.Stop()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-w.stopCh:
|
||||
return
|
||||
case <-ticker.C:
|
||||
alive, known := w.probe(w.parentPID)
|
||||
if !known || alive {
|
||||
continue
|
||||
}
|
||||
launcherInfof("parent process exited; shutting down supervised processes (parent_pid=%d)", w.parentPID)
|
||||
shutdownDebugf("launcher parent watchdog detected exit parent_pid=%d launcher_pid=%d", w.parentPID, os.Getpid())
|
||||
w.shutdown("parent process exit")
|
||||
w.exit(0)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,100 @@
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestParentWatchdogShutsDownWhenParentDies(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
var alive atomic.Bool
|
||||
alive.Store(true)
|
||||
var shutdowns atomic.Int32
|
||||
var exits atomic.Int32
|
||||
watchdog := newParentWatchdog(1234, func(string) {
|
||||
shutdowns.Add(1)
|
||||
}, func(code int) {
|
||||
if code == 0 {
|
||||
exits.Add(1)
|
||||
}
|
||||
})
|
||||
watchdog.interval = time.Second
|
||||
watchdog.probe = func(int64) (bool, bool) {
|
||||
return alive.Load(), true
|
||||
}
|
||||
t.Cleanup(watchdog.stop)
|
||||
|
||||
watchdog.start()
|
||||
time.Sleep(time.Second)
|
||||
synctest.Wait()
|
||||
if shutdowns.Load() != 0 {
|
||||
t.Fatal("watchdog shut down while parent was alive")
|
||||
}
|
||||
|
||||
alive.Store(false)
|
||||
time.Sleep(time.Second)
|
||||
synctest.Wait()
|
||||
if shutdowns.Load() != 1 {
|
||||
t.Fatalf("shutdown count = %d, want 1", shutdowns.Load())
|
||||
}
|
||||
if exits.Load() != 1 {
|
||||
t.Fatalf("exit count = %d, want 1", exits.Load())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestParentWatchdogIgnoresUnknownLiveness(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
var shutdowns atomic.Int32
|
||||
var exits atomic.Int32
|
||||
watchdog := newParentWatchdog(1234, func(string) {
|
||||
shutdowns.Add(1)
|
||||
}, func(int) {
|
||||
exits.Add(1)
|
||||
})
|
||||
watchdog.interval = time.Second
|
||||
watchdog.probe = func(int64) (bool, bool) {
|
||||
return false, false
|
||||
}
|
||||
t.Cleanup(watchdog.stop)
|
||||
|
||||
watchdog.start()
|
||||
time.Sleep(3 * time.Second)
|
||||
synctest.Wait()
|
||||
if shutdowns.Load() != 0 || exits.Load() != 0 {
|
||||
t.Fatalf("unknown liveness caused shutdown=%d exit=%d", shutdowns.Load(), exits.Load())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestParentWatchdogStopIsIdempotent(t *testing.T) {
|
||||
watchdog := newParentWatchdog(1234, func(string) {
|
||||
t.Fatal("unexpected shutdown")
|
||||
}, func(int) {
|
||||
t.Fatal("unexpected exit")
|
||||
})
|
||||
watchdog.probe = func(int64) (bool, bool) { return true, true }
|
||||
watchdog.start()
|
||||
watchdog.stop()
|
||||
watchdog.stop()
|
||||
}
|
||||
|
||||
func TestParentPIDFromEnvRejectsInvalidValues(t *testing.T) {
|
||||
for _, value := range []string{"", "0", "-1", "not-a-pid"} {
|
||||
t.Run(value, func(t *testing.T) {
|
||||
t.Setenv(parentPIDEnv, value)
|
||||
if got := parentPIDFromEnv(); got != 0 {
|
||||
t.Fatalf("parentPIDFromEnv() = %d for %q, want 0", got, value)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParentPIDFromEnvAcceptsPositivePID(t *testing.T) {
|
||||
t.Setenv(parentPIDEnv, "1234")
|
||||
if got := parentPIDFromEnv(); got != 1234 {
|
||||
t.Fatalf("parentPIDFromEnv() = %d, want 1234", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,57 @@
|
||||
//go:build !windows
|
||||
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"os/exec"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestParentWatchdogDetectsReapedProcess(t *testing.T) {
|
||||
child := exec.Command("sleep", "10")
|
||||
if err := child.Start(); err != nil {
|
||||
t.Fatalf("start child: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
if child.ProcessState == nil {
|
||||
_ = child.Process.Kill()
|
||||
}
|
||||
_ = child.Wait()
|
||||
})
|
||||
|
||||
shutdown := make(chan string, 1)
|
||||
exited := make(chan int, 1)
|
||||
watchdog := newParentWatchdog(int64(child.Process.Pid), func(reason string) {
|
||||
shutdown <- reason
|
||||
}, func(code int) {
|
||||
exited <- code
|
||||
})
|
||||
watchdog.interval = 5 * time.Millisecond
|
||||
watchdog.start()
|
||||
t.Cleanup(watchdog.stop)
|
||||
|
||||
if err := child.Process.Kill(); err != nil {
|
||||
t.Fatalf("kill child: %v", err)
|
||||
}
|
||||
if err := child.Wait(); err == nil {
|
||||
t.Fatal("child exited successfully after kill")
|
||||
}
|
||||
|
||||
select {
|
||||
case reason := <-shutdown:
|
||||
if reason != "parent process exit" {
|
||||
t.Fatalf("shutdown reason = %q, want parent process exit", reason)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("watchdog did not detect the reaped process")
|
||||
}
|
||||
select {
|
||||
case code := <-exited:
|
||||
if code != 0 {
|
||||
t.Fatalf("exit code = %d, want 0", code)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("watchdog did not request launcher exit")
|
||||
}
|
||||
}
|
||||
@@ -18,6 +18,11 @@ var (
|
||||
attachSignalsFn = func(supervisor *processSupervisor) {
|
||||
supervisor.attachSignals()
|
||||
}
|
||||
startParentWatchFn = func(supervisor *processSupervisor) *parentWatchdog {
|
||||
watchdog := newParentWatchdogFromEnv(supervisor.shutdown, launcherExit)
|
||||
watchdog.start()
|
||||
return watchdog
|
||||
}
|
||||
)
|
||||
|
||||
func runStart(ctx context.Context, opts Options) int {
|
||||
@@ -92,6 +97,10 @@ func runManagedApp(ctx context.Context, cfg managedAppConfig) int {
|
||||
supervisor := newSupervisorFn()
|
||||
attachSignalsFn(supervisor)
|
||||
shutdownDebugf("runManagedApp signal handler attached")
|
||||
parentWatchdog := startParentWatchFn(supervisor)
|
||||
if parentWatchdog != nil {
|
||||
defer parentWatchdog.stop()
|
||||
}
|
||||
healthToken, err := launchHealthToken()
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
|
||||
|
||||
@@ -168,11 +168,13 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
oldNewSupervisor := newSupervisorFn
|
||||
oldLaunchBackend := launchBackendFn
|
||||
oldAttachSignals := attachSignalsFn
|
||||
oldStartParentWatch := startParentWatchFn
|
||||
oldWaitForHealth := waitForHealthFn
|
||||
t.Cleanup(func() {
|
||||
newSupervisorFn = oldNewSupervisor
|
||||
launchBackendFn = oldLaunchBackend
|
||||
attachSignalsFn = oldAttachSignals
|
||||
startParentWatchFn = oldStartParentWatch
|
||||
waitForHealthFn = oldWaitForHealth
|
||||
})
|
||||
|
||||
@@ -201,6 +203,10 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
attachSignalsFn = func(_ *processSupervisor) {
|
||||
events = append(events, "attach-signals")
|
||||
}
|
||||
startParentWatchFn = func(_ *processSupervisor) *parentWatchdog {
|
||||
events = append(events, "start-parent-watch")
|
||||
return newParentWatchdog(0, nil, nil)
|
||||
}
|
||||
waitForHealthFn = func(_ context.Context, _ string, _ childState, _ time.Duration, expectedToken string, _ func()) error {
|
||||
waitedHealthToken = expectedToken
|
||||
events = append(events, "wait-health")
|
||||
@@ -222,7 +228,7 @@ func TestRunManagedAppAttachesSignalsBeforeBackendLaunch(t *testing.T) {
|
||||
if code != 0 {
|
||||
t.Fatalf("runManagedApp() = %d, want 0", code)
|
||||
}
|
||||
want := []string{"new-supervisor", "attach-signals", "launch-backend", "wait-health"}
|
||||
want := []string{"new-supervisor", "attach-signals", "start-parent-watch", "launch-backend", "wait-health"}
|
||||
if !reflect.DeepEqual(events, want) {
|
||||
t.Fatalf("events = %v, want %v", events, want)
|
||||
}
|
||||
|
||||
@@ -1,9 +0,0 @@
|
||||
//go:build windows
|
||||
|
||||
package tempartifacts
|
||||
|
||||
func processAlive(pid int64) (alive, known bool) {
|
||||
// Windows does not expose a portable, permission-safe signal-zero probe.
|
||||
// Unknown liveness is treated as protected by reconciliation.
|
||||
return false, false
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/proclive"
|
||||
"github.com/kandev/kandev/internal/system/storage"
|
||||
)
|
||||
|
||||
@@ -206,7 +207,7 @@ func (r *Registry) Reconcile(ctx context.Context) error {
|
||||
}
|
||||
continue
|
||||
}
|
||||
alive, known := processAlive(artifact.OwnerPID)
|
||||
alive, known := proclive.Alive(artifact.OwnerPID)
|
||||
if !known || alive {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/proclive"
|
||||
"github.com/kandev/kandev/internal/system/storage"
|
||||
)
|
||||
|
||||
@@ -116,7 +117,7 @@ func TestRegistryReconcilesPreviousRunsAndDeadOwners(t *testing.T) {
|
||||
t.Fatalf("create current-run artifact: %v", err)
|
||||
}
|
||||
deadPID := int64(os.Getpid()) + 1_000_000
|
||||
alive, known := processAlive(deadPID)
|
||||
alive, known := proclive.Alive(deadPID)
|
||||
canCheckDead := known && !alive
|
||||
if canCheckDead {
|
||||
store.artifacts[filepath.Join(root, "kandev-host-utility-dead")] = storage.TemporaryArtifact{
|
||||
|
||||
@@ -24,6 +24,7 @@ const DEFAULT_DESKTOP_PORT: u16 = 38430;
|
||||
const DESKTOP_PORT_ENV: &str = "KANDEV_DESKTOP_PORT";
|
||||
const DESKTOP_HEALTH_TOKEN_ENV: &str = "KANDEV_DESKTOP_HEALTH_TOKEN";
|
||||
const DESKTOP_NATIVE_NOTIFICATIONS_ENV: &str = "KANDEV_DESKTOP_NATIVE_NOTIFICATIONS";
|
||||
const LAUNCHER_PARENT_PID_ENV: &str = "KANDEV_LAUNCHER_PARENT_PID";
|
||||
const DESKTOP_HEALTH_TOKEN_HEADER: &str = "x-kandev-desktop-health-token";
|
||||
const STARTUP_OUTPUT_LIMIT: usize = 12 * 1024;
|
||||
const HEALTH_READY_SETTLE: Duration = Duration::from_millis(100);
|
||||
@@ -269,6 +270,7 @@ fn launch_and_wait(app: &AppHandle, state: &BackendState) -> Result<String, Stri
|
||||
OsString::from(DESKTOP_HEALTH_TOKEN_ENV),
|
||||
OsString::from(&health_token),
|
||||
);
|
||||
add_launcher_parent_pid(&mut inherited_env);
|
||||
let spec = build_backend_command(
|
||||
&runtime_dir,
|
||||
port,
|
||||
@@ -288,6 +290,13 @@ fn launch_and_wait(app: &AppHandle, state: &BackendState) -> Result<String, Stri
|
||||
Ok(format!("http://{LOOPBACK_HOST}:{port}/"))
|
||||
}
|
||||
|
||||
fn add_launcher_parent_pid(env: &mut BTreeMap<OsString, OsString>) {
|
||||
env.insert(
|
||||
OsString::from(LAUNCHER_PARENT_PID_ENV),
|
||||
OsString::from(std::process::id().to_string()),
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(feature = "desktop-runtime")]
|
||||
fn resolve_runtime_dir(app: &AppHandle) -> Result<PathBuf, String> {
|
||||
if let Some(dir) = env::var_os("KANDEV_DESKTOP_RUNTIME_DIR") {
|
||||
@@ -742,6 +751,10 @@ mod tests {
|
||||
OsString::from(DESKTOP_HEALTH_TOKEN_ENV),
|
||||
OsString::from("health-token"),
|
||||
);
|
||||
inherited.insert(
|
||||
OsString::from(LAUNCHER_PARENT_PID_ENV),
|
||||
OsString::from("12345"),
|
||||
);
|
||||
inherited.insert(OsString::from("PATH"), OsString::from("/existing/bin"));
|
||||
|
||||
let spec = build_backend_command(
|
||||
@@ -780,6 +793,10 @@ mod tests {
|
||||
spec.env.get(OsStr::new(DESKTOP_HEALTH_TOKEN_ENV)),
|
||||
Some(&OsString::from("health-token"))
|
||||
);
|
||||
assert_eq!(
|
||||
spec.env.get(OsStr::new(LAUNCHER_PARENT_PID_ENV)),
|
||||
Some(&OsString::from("12345"))
|
||||
);
|
||||
assert_eq!(
|
||||
spec.env
|
||||
.get(OsStr::new("KANDEV_DESKTOP_NATIVE_NOTIFICATIONS")),
|
||||
@@ -787,6 +804,18 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn launcher_parent_environment_uses_shell_pid() {
|
||||
let mut inherited = BTreeMap::new();
|
||||
|
||||
add_launcher_parent_pid(&mut inherited);
|
||||
|
||||
assert_eq!(
|
||||
inherited.get(OsStr::new(LAUNCHER_PARENT_PID_ENV)),
|
||||
Some(&OsString::from(std::process::id().to_string()))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn desktop_environment_preserves_path_and_adds_gui_paths() {
|
||||
let runtime_dir = Path::new("/opt/kandev");
|
||||
|
||||
@@ -0,0 +1,342 @@
|
||||
---
|
||||
spec: docs/specs/desktop-tauri-app/spec.md
|
||||
created: 2026-08-19
|
||||
status: completed
|
||||
---
|
||||
|
||||
# Implementation Plan: Orphaned desktop backend holds the Kandev home lock
|
||||
|
||||
## Overview
|
||||
|
||||
Quitting the macOS desktop app can leave its launcher and backend running. The
|
||||
orphaned backend keeps the `flock` on `~/.kandev/.kandev-backend.lock`, so every
|
||||
later start, desktop or CLI, is rejected with a message that names only the home
|
||||
path and gives the operator nothing to act on. This plan fixes both halves: the
|
||||
launcher learns to exit when the shell that spawned it dies, and the ownership
|
||||
lock learns to say who is holding it.
|
||||
|
||||
Order is bottom-up. A shared process-liveness helper lands first because both
|
||||
halves need it, then the two independent behaviors, then the desktop change that
|
||||
activates the watchdog. Two specs are amended by this repair:
|
||||
[desktop-tauri-app](../../specs/desktop-tauri-app/spec.md) owns the launcher
|
||||
lifetime guarantee, and
|
||||
[port-collision-safety](../../specs/port-collision-safety/spec.md) owns the
|
||||
runtime-state ownership contract.
|
||||
|
||||
## Root cause
|
||||
|
||||
Confirmed on a live machine, not inferred.
|
||||
|
||||
The desktop shell owns backend shutdown through one path: `shutdown_and_exit` in
|
||||
`apps/desktop/src-tauri/src/main.rs` (reached from `WindowEvent::CloseRequested`
|
||||
and `MenuAction::Quit`) calls `BackendState::stop`, which sends `SIGTERM` to the
|
||||
launcher and escalates to `SIGKILL` after five seconds. That path is correct and
|
||||
is not what failed. The defect is that it is the *only* path: nothing binds the
|
||||
launcher's lifetime to the shell, so when the Tauri process ends without running
|
||||
it, the launcher survives.
|
||||
|
||||
Observed state after a quit: launcher PID 51228
|
||||
(`Kandev.app/Contents/Resources/kandev/bin/kandev --headless --port 38430`) alive
|
||||
with **PPID 1**, having been reparented to `launchd` when its Tauri parent died;
|
||||
backend PID 51229 alive as its child; `lsof` showing 51229 holding
|
||||
`/Users/cfl12/.kandev/.kandev-backend.lock`; and no `Kandev.app/Contents/MacOS`
|
||||
process at all. `SIGTERM` to 51228 shut the tree down cleanly and freed the lock,
|
||||
confirming the launcher's own signal handling was never the problem.
|
||||
|
||||
Two consequences compound into the reported dead end:
|
||||
|
||||
1. `apps/backend/internal/launcher/` has no parent-death detection. Neither
|
||||
`runManagedApp` (`start.go`) nor `processSupervisor` (`process.go`) observes
|
||||
the parent, so an orphaned launcher runs until it is killed by hand.
|
||||
2. `ownershiplock` never writes to the lock file it holds, so the conflict
|
||||
message in `apps/backend/internal/backendapp/main.go` can only name the path.
|
||||
The file is zero bytes, which also makes it look deletable; deleting it does
|
||||
nothing, because the `flock` is the ownership, not the file.
|
||||
|
||||
Smallest reliable reproduction: start the desktop app, `kill -9` the Tauri shell
|
||||
process (simulating a crash or force quit), then run `make start-debug`. It fails
|
||||
with `home target "..." is already owned by another backend`.
|
||||
|
||||
Regression-test level: Go unit and real-subprocess tests in
|
||||
`internal/launcher` and `internal/backendapp/ownershiplock`, plus a Rust unit
|
||||
test in `apps/desktop/src-tauri`. No E2E; see Tests.
|
||||
|
||||
---
|
||||
|
||||
## Contracts introduced
|
||||
|
||||
Both halves are new cross-component contracts. Naming them here so the task files
|
||||
do not each re-invent them.
|
||||
|
||||
### `KANDEV_LAUNCHER_PARENT_PID`
|
||||
|
||||
The spawning process writes its own PID into this environment variable. A
|
||||
launcher that sees a valid positive PID binds its lifetime to that process and
|
||||
shuts its supervised tree down once the process is gone. Unset, empty, `0`, or
|
||||
unparseable means no watchdog, which keeps every current invocation, including
|
||||
`make dev`, plain `kandev start` in a terminal, `nohup`, and service-manager
|
||||
installs, behaving exactly as it does today.
|
||||
|
||||
Opt-in is deliberate. Deriving the parent from `os.Getppid()` drift would also
|
||||
kill deliberately detached CLI launches (`nohup kandev start &`), and under
|
||||
`launchd`/`systemd` the initial parent is already PID 1, so there is no stable
|
||||
signal to compare against. The spawner is the only party that knows it owns the
|
||||
launcher's lifetime, so the spawner declares it.
|
||||
|
||||
This variable is launcher-scoped and must **not** be added to the
|
||||
`allowedSupervisorEnv` allowlist in `apps/backend/internal/launcher/supervisor.go`.
|
||||
That allowlist is the environment replayed into a restarted *backend* child; the
|
||||
watchdog belongs to the launcher process, which survives supervisor restarts.
|
||||
|
||||
### Lock owner metadata
|
||||
|
||||
After acquiring a lock, the owner writes one line of JSON into the lock file it
|
||||
already holds:
|
||||
|
||||
```json
|
||||
{"pid":51229,"executable":"/Applications/Kandev.app/.../bin/kandev","started_at":"2026-08-19T09:14:35.123456789Z"}
|
||||
```
|
||||
|
||||
Written with `Truncate(0)` then `WriteAt(1)`, leaving the first byte reserved for
|
||||
the Windows lock range so a conflicting process can read the metadata through a
|
||||
separate handle while the lock is held. A concurrent reader sees either empty
|
||||
or a valid prefix, never a stale tail from a longer previous record. It is
|
||||
advisory only: never read to decide ownership, staleness, or takeover, and a
|
||||
write failure never fails acquisition. The lock sidecar is opened with
|
||||
platform no-follow semantics and must be a regular file; symlink or reparse
|
||||
point paths are rejected before any write.
|
||||
|
||||
---
|
||||
|
||||
## Backend
|
||||
|
||||
### Shared process liveness (`internal/common/proclive`, new)
|
||||
|
||||
`apps/backend/internal/system/storage/tempartifacts/process_alive_unix.go` and
|
||||
`process_alive_windows.go` already implement exactly the probe both halves of
|
||||
this fix need, including the `(alive, known bool)` shape that distinguishes "the
|
||||
process is gone" from "this platform cannot tell". Two more copies would violate
|
||||
the repo's rule against duplicating cross-package helpers, so promote it to
|
||||
`internal/common/proclive` and migrate `tempartifacts` onto it.
|
||||
|
||||
The signature stays `Alive(pid int64) (alive, known bool)`. Keeping `int64`
|
||||
preserves `tempartifacts`' existing `artifact.OwnerPID` call site byte for byte
|
||||
and avoids a 32-bit truncation question at the boundary; the two new callers pass
|
||||
`int64(pid)`.
|
||||
|
||||
Behavior is unchanged from the current implementation: Unix uses signal 0,
|
||||
treating `EPERM` as alive and `ESRCH` as gone; Windows returns `(false, false)`,
|
||||
meaning unknown. Both new callers must therefore be correct when liveness is
|
||||
unknown, and both are: the watchdog only acts on a positive `(false, true)`, and
|
||||
the lock reader only suppresses owner detail on a positive `(false, true)`.
|
||||
|
||||
### Launcher parent watchdog (`internal/launcher/parentwatch.go`, new)
|
||||
|
||||
A `parentWatchdog` owning one goroutine, following the package's existing
|
||||
lifecycle conventions: `start` registers on a `sync.WaitGroup`, `stop` closes a
|
||||
channel and waits for drain, and the poll loop uses `time.NewTicker` inside a
|
||||
`select` that also watches the stop channel, never `time.Sleep`.
|
||||
|
||||
On a poll where the watched PID is positively gone, it mirrors what
|
||||
`attachSignals` already does for a signal: emit an operator-visible
|
||||
`launcherInfof` line and a `shutdownDebugf` line, call
|
||||
`supervisor.shutdown(reason)`, then `launcherExit(0)`. Reusing that exact
|
||||
sequence means the watchdog inherits the tested graceful-shutdown path
|
||||
(`managedProcessShutdownGrace`, process-group cleanup) rather than introducing a
|
||||
second way to stop the tree.
|
||||
|
||||
Poll interval 2 seconds, as a package constant next to the existing shutdown
|
||||
timing constants. Injectable seams for tests: the liveness probe function and the
|
||||
ticker interval.
|
||||
|
||||
Wiring: `runManagedApp` in `apps/backend/internal/launcher/start.go`, immediately
|
||||
after `attachSignalsFn(supervisor)`, through a package-level `var` in the same
|
||||
style as the existing `attachSignalsFn` indirection so tests can substitute it.
|
||||
`runManagedApp` covers `start`, `run`, and `service`; the desktop spawns
|
||||
`kandev --headless --port <n>`, which is `start`. `dev.go` is intentionally not
|
||||
wired: `make dev` is a foreground developer command whose parent is `make`, and
|
||||
no spawner sets the variable for it.
|
||||
|
||||
The env var name belongs in `apps/backend/internal/launcher/constants.go`
|
||||
alongside the other launcher constants.
|
||||
|
||||
### Ownership-lock owner diagnostics (`internal/backendapp/ownershiplock/`)
|
||||
|
||||
`owner.go` (new) holds the record struct plus `writeOwner(*os.File)` and
|
||||
`readOwner(path string)`. `lock.go` gains a `writeOwner` call after `lockFile`
|
||||
succeeds, ignoring its error, and a `readOwner` call on the `ErrConflict` branch
|
||||
to populate a new `Owner *OwnerRecord` field on `ConflictError`.
|
||||
|
||||
`ConflictError.Error()` extends its existing sentence rather than replacing it,
|
||||
so the message keeps naming the path and the existing
|
||||
`use a separate KANDEV_HOME_DIR` guidance appended in `main.go` still reads
|
||||
correctly:
|
||||
|
||||
```text
|
||||
home target "/Users/cfl12/.kandev" is already owned by another backend (pid 51229, /Applications/Kandev.app/.../bin/kandev, started 2026-08-19T09:14:35Z)
|
||||
```
|
||||
|
||||
Owner detail is omitted, falling back to today's exact wording, when the file is
|
||||
empty or unparseable, or when `proclive.Alive` positively reports the recorded
|
||||
PID as gone. It is included when liveness is unknown, which is what makes the
|
||||
detail useful on Windows.
|
||||
|
||||
No change is needed in `apps/backend/internal/backendapp/main.go`: it already
|
||||
prints `%v` of the error, so the enriched text reaches stderr, and from there
|
||||
reaches the desktop startup-error dialog through the launcher output capture in
|
||||
`apps/desktop/src-tauri/src/backend.rs`.
|
||||
|
||||
---
|
||||
|
||||
## Desktop
|
||||
|
||||
### `apps/desktop/src-tauri/src/backend.rs`
|
||||
|
||||
`launch_and_wait` already inserts `KANDEV_DESKTOP_HEALTH_TOKEN` into
|
||||
`inherited_env` before building the spec. Insert `KANDEV_LAUNCHER_PARENT_PID`
|
||||
set to `std::process::id()` the same way, one line, next to the existing token
|
||||
insert. `desktop_environment` passes unrecognized inherited variables through
|
||||
untouched, which the existing `CUSTOM_ENV` assertion in
|
||||
`command_spec_uses_headless_launcher_and_loopback_env` already pins.
|
||||
|
||||
No Rust logic changes anywhere else. `BackendState::stop` remains the primary
|
||||
shutdown path for a normal quit; the watchdog is the backstop for the abnormal
|
||||
exits `stop` cannot cover.
|
||||
|
||||
---
|
||||
|
||||
## Frontend
|
||||
|
||||
None. This repair has no SPA surface: no component, store slice, API client, or
|
||||
user-facing copy changes, so no i18n work either. The one operator-visible string
|
||||
is backend stderr, which stays English per the backend i18n scope in
|
||||
`apps/backend/AGENTS.md`.
|
||||
|
||||
---
|
||||
|
||||
## Tests
|
||||
|
||||
| What | File | How |
|
||||
|---|---|---|
|
||||
| `Alive` reports self alive, reports a reaped PID gone, treats non-positive PIDs as gone-and-known | `internal/common/proclive/proclive_unix_test.go` | Separate Unix tests; reap a real short-lived `os/exec` child for the dead case |
|
||||
| `Alive` reports unknown on Windows | `internal/common/proclive/proclive_windows_test.go` | Windows-only test asserts `(false, false)` for the current process |
|
||||
| `tempartifacts` keeps its current reconciliation behavior after migrating | `internal/system/storage/tempartifacts/registry_test.go` | Existing tests must pass unchanged |
|
||||
| Watchdog shuts the tree down when the watched PID dies | `internal/launcher/parentwatch_test.go` | `testing/synctest` with an injected liveness probe flipping to `(false, true)`; assert `supervisor.shutdown` and `launcherExit(0)` via the package's existing `launcherExit` seam |
|
||||
| Watchdog stays quiet while the parent lives, and while liveness is unknown | `internal/launcher/parentwatch_test.go` | Same harness with probes returning `(true, true)` and `(false, false)`; assert no shutdown after several ticks |
|
||||
| Unset/empty/`0`/unparseable env means no watchdog goroutine | `internal/launcher/parentwatch_test.go` | Table-driven over env values; assert `start` returns a no-op and `stop` is safe |
|
||||
| `stop` drains the goroutine and is idempotent | `internal/launcher/parentwatch_test.go` | Call `stop` twice; assert no panic and no goroutine left |
|
||||
| Watchdog fires against a real killed process | `internal/launcher/parentwatch_unix_test.go` | Spawn a real child, kill and reap it, assert the probe path observes it gone |
|
||||
| Conflict names the owning PID and executable | `internal/backendapp/ownershiplock/lock_test.go` | Acquire in-process, attempt a second acquire, assert `ConflictError.Owner` and the rendered message |
|
||||
| Empty, unparseable, and known-dead metadata fall back to the path-only message | `internal/backendapp/ownershiplock/lock_test.go` | Table-driven over lock-file contents with an injected liveness probe |
|
||||
| Owner-metadata write failure does not fail acquisition | `internal/backendapp/ownershiplock/lock_test.go` | Force the write to fail; assert `Acquire` still returns an owner |
|
||||
| Metadata write truncates a longer previous record | `internal/backendapp/ownershiplock/owner_test.go` | Pre-fill the file with a longer record, re-write, assert no trailing bytes |
|
||||
| Existing startup-conflict behavior is unchanged | `internal/backendapp/main_test.go` | Existing `TestBackendStartupConflictStopsBeforeSharedStateInitialization` must pass unchanged |
|
||||
| Desktop spec carries the parent PID to the launcher | `apps/desktop/src-tauri/src/backend.rs` (`mod tests`) | Extend the existing command-spec test to assert the variable survives into `BackendCommandSpec.env` |
|
||||
|
||||
Commands, each runnable from the repo root:
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/common/proclive/... ./internal/launcher/... ./internal/backendapp/... ./internal/system/storage/tempartifacts/...
|
||||
cd apps/desktop/src-tauri && cargo test --features desktop-runtime
|
||||
```
|
||||
|
||||
Lint gate for the changed Go packages:
|
||||
|
||||
```bash
|
||||
cd apps/backend && golangci-lint run ./internal/common/proclive/... ./internal/launcher/... ./internal/backendapp/... --new-from-rev=origin/main --timeout=5m
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## E2E Tests
|
||||
|
||||
None, deliberately. The spec changes have no SPA surface, so there is no
|
||||
Playwright scenario to write. The desktop smoke harness
|
||||
(`pnpm --filter @kandev/desktop e2e`) runs under Xvfb on Linux and would have to
|
||||
abnormally kill the shell and then assert on host process state and a host lock
|
||||
file, which is exactly the coupling that harness avoids. The real-subprocess Go
|
||||
test in `internal/launcher` covers the mechanism, and the Rust unit test covers
|
||||
the wiring that activates it.
|
||||
|
||||
---
|
||||
|
||||
## Verification Results
|
||||
|
||||
- `cd apps/backend && go test -race ./internal/common/proclive/... ./internal/launcher/... ./internal/backendapp/... ./internal/system/storage/tempartifacts/...`
|
||||
passed for all six affected packages.
|
||||
- `cd apps/backend && gofmt -l internal/common/proclive internal/launcher internal/backendapp/ownershiplock internal/system/storage/tempartifacts`
|
||||
produced no output.
|
||||
- `cd apps/backend && golangci-lint run ./internal/common/proclive/... ./internal/launcher/... ./internal/backendapp/... ./internal/system/storage/tempartifacts/... --new-from-rev=origin/main --timeout=5m`
|
||||
passed with 0 issues.
|
||||
- `GOOS=windows GOARCH=amd64 go test -c` passed for `proclive`, `launcher`, and
|
||||
`backendapp/ownershiplock`; temporary Windows test binaries were removed.
|
||||
- `cd apps/desktop/src-tauri && cargo fmt --check && cargo test --features desktop-runtime`
|
||||
passed: 57 tests, 0 failures.
|
||||
- macOS parent-death process-boundary verification passed with a newly built
|
||||
launcher and the installed runtime bundle: after SIGKILLing and reaping the
|
||||
declared parent shell, the launcher logged `parent process exited`, completed
|
||||
graceful shutdown, exited, and the temporary runtime lock had no holder.
|
||||
- `git diff --check` passed. Generated Tauri schema output from the test build
|
||||
was removed; no generated artifact remains in the diff.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Waves And Parallel Candidates
|
||||
|
||||
```text
|
||||
Wave 1:
|
||||
- [x] [task-01-proclive-shared-helper](task-01-proclive-shared-helper.md)
|
||||
|
||||
Wave 2 (parallel candidates, user authorization required):
|
||||
- [x] [task-02-launcher-parent-watchdog](task-02-launcher-parent-watchdog.md)
|
||||
- [x] [task-04-lock-owner-diagnostics](task-04-lock-owner-diagnostics.md)
|
||||
|
||||
Wave 3:
|
||||
- [x] [task-03-desktop-parent-pid](task-03-desktop-parent-pid.md)
|
||||
```
|
||||
|
||||
Task 01 is a shared dependency of both wave-2 tasks and must land first. Tasks 02
|
||||
and 04 touch disjoint packages (`internal/launcher` and
|
||||
`internal/backendapp/ownershiplock`), share no schema, migration, generated
|
||||
contract, lockfile, or package config, and are genuinely parallel-safe. Task 03
|
||||
is last because it activates the watchdog task 02 introduces, and the product
|
||||
should not advertise the binding before the launcher honors it.
|
||||
|
||||
Waves are a review aid. The default remains sequential execution in the primary
|
||||
conversation; they do not authorize subagents.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Contradicts a rejected ADR alternative.** ADR
|
||||
[2026-08-09-exclusive-runtime-state-ownership](../../decisions/2026-08-09-exclusive-runtime-state-ownership.md)
|
||||
rejected "Use a PID file" because stale PIDs and PID reuse do not provide
|
||||
atomic ownership, and `port-collision-safety` listed PID identity matching as
|
||||
out of scope. Both objections are about using a PID to *decide* ownership. This
|
||||
plan keeps `flock` as the sole ownership proof and uses the PID only as text in
|
||||
an error message, and the spec amendment narrows the out-of-scope line to say
|
||||
so explicitly. Reviewers should confirm that framing is acceptable before the
|
||||
code lands; if it is not, task 04 drops and task 02 still fixes the reported
|
||||
dead end.
|
||||
- **PID reuse weakens the watchdog silently.** If the shell dies and its PID is
|
||||
reused before the next poll, the watchdog sees a live PID and never fires. The
|
||||
failure mode is exactly today's behavior, so it degrades rather than breaks. A
|
||||
handle-based or pipe-EOF mechanism would close it and is the natural follow-up
|
||||
if this proves insufficient in practice.
|
||||
- **Windows gets the watchdog as a no-op**, because `proclive.Alive` cannot
|
||||
determine liveness there. Windows also has no reparent-to-PID-1 semantics, and
|
||||
the reported bug is macOS, so this is scoped rather than solved. The lock
|
||||
diagnostics do work on Windows, since unknown liveness still reports the
|
||||
recorded values.
|
||||
- **A 2 second poll leaves a window** where a restarted desktop still sees the
|
||||
old lock. The new error message names the owning PID, which makes that window
|
||||
self-explanatory instead of mysterious.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing `flock` for a different ownership primitive, or any takeover,
|
||||
lock-breaking, or stale-lock-reclaim behavior.
|
||||
- Wiring the watchdog into `dev.go`, or changing `make dev` process supervision.
|
||||
- Any UI affordance for detecting or killing a conflicting backend from the
|
||||
desktop startup-error screen.
|
||||
- Changing `BackendState::stop`, the health-token contract, or the desktop
|
||||
updater restart path.
|
||||
@@ -0,0 +1,124 @@
|
||||
---
|
||||
id: "01-proclive-shared-helper"
|
||||
title: "Shared process-liveness helper"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/desktop-tauri-app/spec.md"
|
||||
parallelism: sequential
|
||||
---
|
||||
|
||||
# Task 01: Shared process-liveness helper
|
||||
|
||||
Promote the existing private `processAlive` probe out of
|
||||
`internal/system/storage/tempartifacts` into `internal/common/proclive` so the
|
||||
launcher watchdog (task 02) and the ownership-lock diagnostics (task 04) consume
|
||||
one implementation instead of adding a second and third copy.
|
||||
|
||||
This is a pure move plus re-export. Do not change the probe's behavior.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Plan section "Shared process liveness (`internal/common/proclive`, new)".
|
||||
- Existing implementation to move:
|
||||
`apps/backend/internal/system/storage/tempartifacts/process_alive_unix.go`
|
||||
and `process_alive_windows.go`.
|
||||
- Its only current call site: `tempartifacts/registry.go:209`.
|
||||
- `apps/backend/AGENTS.md`, "Cross-tier shared code belongs in `internal/common/`",
|
||||
which is why this lands as a shared package rather than being duplicated.
|
||||
|
||||
## Implementation
|
||||
|
||||
Create `apps/backend/internal/common/proclive/` with:
|
||||
|
||||
- `proclive.go` holding only the package doc comment. State that `known=false`
|
||||
means the platform cannot determine liveness and that callers must treat that
|
||||
as "do not act", since both consumers depend on that reading.
|
||||
- `proclive_unix.go` (`//go:build !windows`) and `proclive_windows.go`
|
||||
(`//go:build windows`), each exporting:
|
||||
|
||||
```go
|
||||
func Alive(pid int64) (alive, known bool)
|
||||
```
|
||||
|
||||
Move the two existing bodies verbatim, changing only the package clause and the
|
||||
identifier from `processAlive` to `Alive`. Keep the `int64` parameter: it matches
|
||||
`tempartifacts`' `artifact.OwnerPID` exactly and avoids a 32-bit truncation
|
||||
question at the boundary. Keep the Windows comment explaining why it returns
|
||||
unknown.
|
||||
|
||||
Then delete both `tempartifacts/process_alive_*.go` files and update
|
||||
`registry.go:209` to call `proclive.Alive(artifact.OwnerPID)`. Update
|
||||
`registry_test.go:119` the same way. Do not change any surrounding
|
||||
reconciliation logic.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `internal/common/proclive` exports `Alive(pid int64) (alive, known bool)` with
|
||||
Unix and Windows implementations, and both `tempartifacts/process_alive_*.go`
|
||||
files are gone.
|
||||
- `tempartifacts` calls `proclive.Alive` and its existing tests pass unchanged.
|
||||
- No behavior change: signal 0 on Unix with `EPERM` treated as alive and `ESRCH`
|
||||
as gone, `(false, false)` on Windows.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/common/proclive/... ./internal/system/storage/tempartifacts/...
|
||||
```
|
||||
|
||||
```bash
|
||||
cd apps/backend && gofmt -l internal/common/proclive internal/system/storage/tempartifacts
|
||||
```
|
||||
|
||||
The `gofmt` invocation must print nothing; the pre-commit hook fails the commit
|
||||
otherwise.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/common/proclive/proclive.go` (new)
|
||||
- `apps/backend/internal/common/proclive/proclive_unix.go` (new)
|
||||
- `apps/backend/internal/common/proclive/proclive_windows.go` (new)
|
||||
- `apps/backend/internal/common/proclive/proclive_test.go` (new)
|
||||
- `apps/backend/internal/system/storage/tempartifacts/process_alive_unix.go` (delete)
|
||||
- `apps/backend/internal/system/storage/tempartifacts/process_alive_windows.go` (delete)
|
||||
- `apps/backend/internal/system/storage/tempartifacts/registry.go`
|
||||
- `apps/backend/internal/system/storage/tempartifacts/registry_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Tests to write
|
||||
|
||||
In `proclive_test.go`, table-driven:
|
||||
|
||||
- The current process (`int64(os.Getpid())`) reports `(true, true)` on Unix.
|
||||
- A reaped child reports `(false, true)` on Unix. Start a real short-lived
|
||||
process with `os/exec`, `Wait` for it so the PID is fully released, then probe.
|
||||
Guard this case with `//go:build !windows` or a `runtime.GOOS` skip, since
|
||||
Windows returns unknown by design.
|
||||
- `0` and a negative PID report `(false, true)` on Unix without touching the
|
||||
platform call.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the moved files, the migrated call sites, the exact commands run with
|
||||
their outcomes, and confirmation that no `tempartifacts` behavior changed. Update
|
||||
this file's `status` and `## Results`, and the matching checkbox in `plan.md`.
|
||||
|
||||
## Results
|
||||
|
||||
- Added `internal/common/proclive` with Unix signal-zero and Windows unknown
|
||||
implementations, plus Unix tests for the current process, a reaped child, and
|
||||
non-positive PIDs.
|
||||
- Migrated `tempartifacts` production and test call sites to `proclive.Alive`;
|
||||
removed the duplicated platform files.
|
||||
- `cd apps/backend && go test ./internal/common/proclive/... ./internal/system/storage/tempartifacts/...`
|
||||
passed (`ok` for both packages).
|
||||
- `gofmt -w internal/common/proclive internal/system/storage/tempartifacts`
|
||||
completed successfully.
|
||||
- Cleanup: the subprocess used by the liveness test is reaped with `Wait`;
|
||||
no temporary capture files or external resources remain.
|
||||
- Security/trust and external side-effect boundaries: none.
|
||||
@@ -0,0 +1,170 @@
|
||||
---
|
||||
id: "02-launcher-parent-watchdog"
|
||||
title: "Launcher parent-death watchdog"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-proclive-shared-helper"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/desktop-tauri-app/spec.md"
|
||||
parallelism: parallel-safe
|
||||
---
|
||||
|
||||
# Task 02: Launcher parent-death watchdog
|
||||
|
||||
Bind the launcher's lifetime to the process that spawned it, so a shell that dies
|
||||
without running its own shutdown path cannot leave the launcher and backend
|
||||
holding the Kandev home lock forever.
|
||||
|
||||
Parallel-safe with task 04: this task touches only `internal/launcher`, task 04
|
||||
touches only `internal/backendapp/ownershiplock`, and they share no schema,
|
||||
migration, generated contract, lockfile, or package config. Both depend on task
|
||||
01.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Plan sections "`KANDEV_LAUNCHER_PARENT_PID`" and "Launcher parent watchdog".
|
||||
- Spec: `docs/specs/desktop-tauri-app/spec.md`, the abnormal-termination bullet
|
||||
under "Failure modes" and its matching scenario under "Scenarios".
|
||||
- Pattern to mirror: `processSupervisor.attachSignals` in
|
||||
`apps/backend/internal/launcher/process.go:141`. The watchdog performs the same
|
||||
terminal sequence as a received signal, so reuse that shape rather than
|
||||
inventing a second way to stop the tree.
|
||||
- Test seams already in the package: `launcherExit` (`process.go:27`) and
|
||||
`launcherStatusOutput` (`process.go:26`); the `attachSignalsFn` indirection in
|
||||
`start.go:18`.
|
||||
- `apps/backend/AGENTS.md`, "Goroutine ownership and leak testing", for the
|
||||
required `start`/`stop` shape.
|
||||
|
||||
## Implementation
|
||||
|
||||
Add the env var name to `apps/backend/internal/launcher/constants.go`:
|
||||
|
||||
```go
|
||||
// parentPIDEnv lets a spawning process bind this launcher's lifetime to its
|
||||
// own. Desktop shells set it so an abnormal shell exit cannot strand a backend
|
||||
// holding the Kandev home's runtime-state lock.
|
||||
parentPIDEnv = "KANDEV_LAUNCHER_PARENT_PID"
|
||||
```
|
||||
|
||||
Add `parentWatchPollInterval = 2 * time.Second` next to the existing shutdown
|
||||
timing constants in `process.go`, or in the new file; keep it a package constant
|
||||
either way.
|
||||
|
||||
Create `apps/backend/internal/launcher/parentwatch.go` with a `parentWatchdog`
|
||||
type owning exactly one goroutine:
|
||||
|
||||
- Construct from the env var. A missing, empty, non-numeric, zero, or negative
|
||||
value yields a watchdog whose `start` and `stop` are no-ops, so every current
|
||||
invocation keeps today's behavior.
|
||||
- `start()` launches the poll goroutine and registers it on a `sync.WaitGroup`.
|
||||
- `stop()` closes a stop channel and waits for drain. Idempotent: safe to call
|
||||
twice.
|
||||
- The loop uses `time.NewTicker(interval)` inside a `select` that also watches
|
||||
the stop channel. Do not use `time.Sleep`.
|
||||
- On a tick, call the liveness probe. Act **only** on `(alive=false, known=true)`.
|
||||
`(false, false)` means the platform cannot tell and must be ignored, which is
|
||||
what makes this a no-op on Windows rather than a process killer.
|
||||
- When the parent is positively gone, emit both an operator-visible
|
||||
`launcherInfof` line naming the watched PID and a `shutdownDebugf` line, then
|
||||
call `supervisor.shutdown(reason)` followed by `launcherExit(0)`.
|
||||
|
||||
Expose two injectable seams as struct fields or package `var`s so tests do not
|
||||
depend on real time or real processes: the liveness probe (defaulting to
|
||||
`proclive.Alive`) and the poll interval.
|
||||
|
||||
Wire it in `runManagedApp` (`apps/backend/internal/launcher/start.go:86`),
|
||||
immediately after `attachSignalsFn(supervisor)`, through a new package-level
|
||||
`var startParentWatchFn = ...` in the same style as the existing
|
||||
`attachSignalsFn`, so tests can substitute it. Stop the watchdog before
|
||||
`runManagedApp` returns.
|
||||
|
||||
Do **not** wire `dev.go`, and do **not** add `KANDEV_LAUNCHER_PARENT_PID` to
|
||||
`allowedSupervisorEnv` in `supervisor.go`. That allowlist is replayed into a
|
||||
restarted backend child; the watchdog belongs to the launcher process, which
|
||||
outlives supervisor restarts. Adding it there would be a real bug.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- With `KANDEV_LAUNCHER_PARENT_PID` set to a live PID, the launcher runs exactly
|
||||
as it does today; when that PID is positively gone, it runs the standard
|
||||
graceful shutdown and exits 0.
|
||||
- With the variable unset, empty, `0`, negative, or unparseable, no goroutine
|
||||
starts and behavior is byte-for-byte unchanged.
|
||||
- An unknown-liveness probe result never triggers shutdown.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/launcher/...
|
||||
```
|
||||
|
||||
```bash
|
||||
cd apps/backend && gofmt -l internal/launcher && golangci-lint run ./internal/launcher/... --new-from-rev=origin/main --timeout=5m
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/launcher/parentwatch.go` (new)
|
||||
- `apps/backend/internal/launcher/parentwatch_test.go` (new)
|
||||
- `apps/backend/internal/launcher/parentwatch_unix_test.go` (new)
|
||||
- `apps/backend/internal/launcher/constants.go`
|
||||
- `apps/backend/internal/launcher/start.go`
|
||||
|
||||
Note `apps/backend/AGENTS.md`'s 800-effective-line file limit, which applies to
|
||||
test files: keep the new tests in the new files above rather than appending to
|
||||
`process_test.go` or `start_test.go`.
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 (`proclive.Alive`).
|
||||
|
||||
## Tests to write
|
||||
|
||||
In `parentwatch_test.go`, using `testing/synctest` so ticker waits resolve
|
||||
instantly, with an injected probe and the `launcherExit` seam captured:
|
||||
|
||||
- Parent dies: probe flips to `(false, true)`; assert the supervisor shut down
|
||||
and `launcherExit` was called with `0`.
|
||||
- Parent lives: probe stays `(true, true)` across several ticks; assert no
|
||||
shutdown and no exit.
|
||||
- Liveness unknown: probe returns `(false, false)` across several ticks; assert
|
||||
no shutdown and no exit. This is the Windows path and the most important
|
||||
negative case.
|
||||
- Env parsing, table-driven over unset, `""`, `"0"`, `"-1"`, `"abc"`, and a
|
||||
valid PID; assert a watchdog is created only for the last, and that `start`
|
||||
and `stop` on the others are safe no-ops.
|
||||
- `stop` is idempotent and drains: call it twice, assert no panic and that the
|
||||
goroutine returned.
|
||||
|
||||
In `parentwatch_unix_test.go` (`//go:build !windows`), one real-process test:
|
||||
spawn a child, kill and `Wait` it so the PID is released, then assert the
|
||||
watchdog's real (non-injected) probe path observes it gone. This is what proves
|
||||
the mechanism against a real OS rather than a fake.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the new files, the wiring point in `start.go`, exact commands with
|
||||
outcomes, and explicit confirmation that `allowedSupervisorEnv` and `dev.go` were
|
||||
left untouched. Update this file's `status` and `## Results`, and the matching
|
||||
checkbox in `plan.md`.
|
||||
|
||||
## Results
|
||||
|
||||
- Added `parentWatchdog` with explicit start/stop ownership, a ticker-driven
|
||||
liveness loop, positive-death-only shutdown, and injectable liveness/timing
|
||||
seams.
|
||||
- Added `KANDEV_LAUNCHER_PARENT_PID` parsing and wired the watchdog into
|
||||
`runManagedApp`; `dev.go` and `allowedSupervisorEnv` remain untouched.
|
||||
- Added fake-time tests for parent death, live parents, unknown liveness, invalid
|
||||
environment values, idempotent stop, and a real Unix subprocess test that
|
||||
kills and reaps the watched process.
|
||||
- `cd apps/backend && go test -run 'TestParentWatchdog|TestParentPIDFromEnv|TestRunManagedAppAttachesSignalsBeforeBackendLaunch' ./internal/launcher/...`
|
||||
passed.
|
||||
- `cd apps/backend && go test ./internal/launcher/...` passed for both launcher
|
||||
packages.
|
||||
- `gofmt -w` completed for all changed launcher Go files.
|
||||
- Cleanup: the real subprocess test kills and reaps its child, and every
|
||||
watchdog registers a cleanup stop; no background watchdog remains.
|
||||
- Security/trust and external side-effect boundaries: the PID is an explicit
|
||||
lifetime declaration from the desktop spawner; unknown liveness never causes
|
||||
shutdown.
|
||||
@@ -0,0 +1,138 @@
|
||||
---
|
||||
id: "03-desktop-parent-pid"
|
||||
title: "Desktop publishes its parent PID to the launcher"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on: ["02-launcher-parent-watchdog"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/desktop-tauri-app/spec.md"
|
||||
parallelism: sequential
|
||||
---
|
||||
|
||||
# Task 03: Desktop publishes its parent PID to the launcher
|
||||
|
||||
Activate the watchdog from task 02 by having the Tauri shell declare its own
|
||||
process ID to the launcher it spawns. This is the change that actually fixes the
|
||||
reported bug; tasks 01 and 02 build the mechanism, this one turns it on.
|
||||
|
||||
Sequential and last: the launcher must already honor
|
||||
`KANDEV_LAUNCHER_PARENT_PID` before the desktop advertises it.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Plan sections "`KANDEV_LAUNCHER_PARENT_PID`" and "Desktop".
|
||||
- Spec: `docs/specs/desktop-tauri-app/spec.md`, the abnormal-termination bullet
|
||||
under "Failure modes" and its matching scenario under "Scenarios".
|
||||
- The exact pattern to copy: `launch_and_wait` in
|
||||
`apps/desktop/src-tauri/src/backend.rs:263`, which already inserts
|
||||
`DESKTOP_HEALTH_TOKEN_ENV` into `inherited_env` before building the spec.
|
||||
- `apps/desktop/AGENTS.md` for the desktop test command and the rule that the
|
||||
shell stays a thin wrapper.
|
||||
|
||||
## Implementation
|
||||
|
||||
In `apps/desktop/src-tauri/src/backend.rs`, add the constant beside the existing
|
||||
desktop env constants near line 24:
|
||||
|
||||
```rust
|
||||
const LAUNCHER_PARENT_PID_ENV: &str = "KANDEV_LAUNCHER_PARENT_PID";
|
||||
```
|
||||
|
||||
In `launch_and_wait`, next to the existing health-token insert, add:
|
||||
|
||||
```rust
|
||||
inherited_env.insert(
|
||||
OsString::from(LAUNCHER_PARENT_PID_ENV),
|
||||
OsString::from(std::process::id().to_string()),
|
||||
);
|
||||
```
|
||||
|
||||
That is the whole production change. `desktop_environment` passes unrecognized
|
||||
inherited variables through untouched, which the existing `CUSTOM_ENV`
|
||||
assertion in `command_spec_uses_headless_launcher_and_loopback_env` already
|
||||
pins, so no plumbing change is needed in `backend_command_spec`.
|
||||
|
||||
Do not change `BackendState::stop`, `terminate_child`, `shutdown_and_exit`, or
|
||||
any `RunEvent` handling. Those remain the primary shutdown path for a normal
|
||||
quit. The watchdog is strictly a backstop for the abnormal exits that path
|
||||
cannot cover, and weakening it would trade one bug for another.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A launcher spawned by the desktop shell receives
|
||||
`KANDEV_LAUNCHER_PARENT_PID` set to the shell's own PID.
|
||||
- Normal quit behavior is unchanged: the shell still terminates its child
|
||||
directly and does not wait for the watchdog.
|
||||
- No other desktop behavior changes.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/desktop/src-tauri && cargo test --features desktop-runtime
|
||||
```
|
||||
|
||||
Manual confirmation of the actual reported bug, on macOS, after tasks 01 to 03
|
||||
have landed and the desktop runtime has been rebuilt:
|
||||
|
||||
1. Start the desktop app and confirm it reaches the SPA.
|
||||
2. Find the Tauri shell process with
|
||||
`pgrep -f "Kandev.app/Contents/MacOS"` and `kill -9` it, simulating the crash
|
||||
or force quit that `BackendState::stop` cannot cover.
|
||||
3. Within roughly the poll interval, confirm
|
||||
`pgrep -f "Kandev.app.*bin/kandev"` returns nothing and
|
||||
`lsof ~/.kandev/.kandev-backend.lock` reports no holder.
|
||||
4. Confirm `make start-debug` then starts normally.
|
||||
|
||||
Record the observed result of step 3, including how long it took, in
|
||||
`## Results`.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/desktop/src-tauri/src/backend.rs`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 02. The launcher must honor the variable before the desktop sets it.
|
||||
|
||||
## Tests to write
|
||||
|
||||
Extend the existing `mod tests` in `backend.rs`: assert that a
|
||||
`KANDEV_LAUNCHER_PARENT_PID` entry present in `inherited_env` survives into
|
||||
`BackendCommandSpec.env` with its value intact. Follow
|
||||
`command_spec_uses_headless_launcher_and_loopback_env`, which already proves the
|
||||
pass-through shape with `CUSTOM_ENV`.
|
||||
|
||||
Assert on the pass-through rather than on `std::process::id()`, so the test does
|
||||
not depend on the PID of the test binary.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the changed file, the exact `cargo test` outcome, and the observed result
|
||||
of the manual macOS orphan check including timing. If the manual check cannot be
|
||||
run on the executing machine, say so explicitly rather than implying it passed.
|
||||
Update this file's `status` and `## Results`, and the matching checkbox in
|
||||
`plan.md`.
|
||||
|
||||
## Results
|
||||
|
||||
- Added `LAUNCHER_PARENT_PID_ENV` and pass the Tauri shell PID into the
|
||||
launcher environment during `launch_and_wait`.
|
||||
- Added a focused Rust test for the PID environment helper and extended the
|
||||
command-spec pass-through coverage.
|
||||
- The first RED run,
|
||||
`cargo test --features desktop-runtime launcher_parent_environment_uses_shell_pid`,
|
||||
failed at compile time because `add_launcher_parent_pid` did not yet exist.
|
||||
- `cd apps/desktop/src-tauri && cargo fmt --check && cargo test --features desktop-runtime`
|
||||
passed: 57 tests, 0 failures; updater verifier and doc-test targets also
|
||||
passed with zero tests.
|
||||
- macOS process-boundary verification passed using a newly built launcher and
|
||||
the installed runtime bundle: a shell parent declared its PID, the launcher
|
||||
reached `/health`, the shell was SIGKILLed and reaped, the launcher exited
|
||||
within the watchdog window, and `lsof` found no holder for the temporary
|
||||
runtime lock. The launcher log recorded `parent process exited` and a
|
||||
graceful shutdown.
|
||||
- Cleanup: the temporary runtime home, database, logs, launcher, backend, and
|
||||
subprocess parent were removed or reaped by the verification command.
|
||||
- Security/trust and external side-effect boundaries: the PID is only passed
|
||||
from the spawning desktop shell to its child launcher; no frontend or network
|
||||
contract changed.
|
||||
@@ -0,0 +1,195 @@
|
||||
---
|
||||
id: "04-lock-owner-diagnostics"
|
||||
title: "Ownership-lock owner diagnostics"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-proclive-shared-helper"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/port-collision-safety/spec.md"
|
||||
parallelism: parallel-safe
|
||||
---
|
||||
|
||||
# Task 04: Ownership-lock owner diagnostics
|
||||
|
||||
Make a runtime-state ownership conflict name the process holding the lock, so an
|
||||
operator with no leftover terminal can identify what to stop instead of being
|
||||
told only that the home is busy.
|
||||
|
||||
Parallel-safe with task 02: this task touches only
|
||||
`internal/backendapp/ownershiplock`, task 02 touches only `internal/launcher`.
|
||||
Both depend on task 01.
|
||||
|
||||
**Read the boundary before writing code.** ADR
|
||||
[2026-08-09-exclusive-runtime-state-ownership](../../decisions/2026-08-09-exclusive-runtime-state-ownership.md)
|
||||
rejected "Use a PID file" as an ownership mechanism, and
|
||||
`docs/specs/port-collision-safety/spec.md` lists PID identity matching as out of
|
||||
scope. This task stays inside that boundary: the `flock` remains the sole
|
||||
ownership proof, and the recorded PID is text in an error message. If you find
|
||||
yourself reading owner metadata to decide whether a lock is held, stale, or
|
||||
takeable, stop. That is the rejected design.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Plan sections "Lock owner metadata" and "Ownership-lock owner diagnostics".
|
||||
- Spec: `docs/specs/port-collision-safety/spec.md`, the "Exclusive runtime-state
|
||||
ownership" paragraphs on advisory owner metadata, its narrowed out-of-scope
|
||||
bullet, and scenarios 6 to 8 under "Concurrent backend startup".
|
||||
- Current code: `apps/backend/internal/backendapp/ownershiplock/lock.go`,
|
||||
especially `Acquire` (line 40), `openLockFile` (line 67), and `ConflictError`
|
||||
(line 15).
|
||||
- Consumer of the message:
|
||||
`apps/backend/internal/backendapp/main.go:220`. It prints `%v`, so no change
|
||||
is required there; confirm that rather than editing it.
|
||||
|
||||
## Implementation
|
||||
|
||||
Create `apps/backend/internal/backendapp/ownershiplock/owner.go` with:
|
||||
|
||||
```go
|
||||
// OwnerRecord is advisory diagnostic metadata about the process that acquired a
|
||||
// lock. It is never consulted to decide ownership, staleness, or takeover: the
|
||||
// operating-system lock is the only proof. See ADR 2026-08-09.
|
||||
type OwnerRecord struct {
|
||||
PID int64 `json:"pid"`
|
||||
Executable string `json:"executable"`
|
||||
StartedAt string `json:"started_at"` // RFC3339Nano
|
||||
}
|
||||
```
|
||||
|
||||
plus `writeOwner(file *os.File) error` and `readOwner(path string) *OwnerRecord`.
|
||||
|
||||
`writeOwner` builds the record from `os.Getpid()`, `os.Executable()` (empty
|
||||
string on error, not a failure), and `time.Now().UTC().Format(time.RFC3339Nano)`,
|
||||
then marshals to a single line. Write it with `file.Truncate(0)` followed by
|
||||
`file.WriteAt(data, ownerMetadataOffset)` on the handle already held. The first
|
||||
byte is reserved for the Windows lock range, so a conflicting process can read
|
||||
the metadata through a separate handle while the lock is held. Truncating first
|
||||
is what guarantees a concurrent reader sees either nothing or a valid prefix,
|
||||
never a stale tail from a longer previous record.
|
||||
|
||||
`readOwner` opens the path separately for reading, reads after the reserved
|
||||
byte, unmarshals, and returns `nil` on any error, on an empty file, or on a
|
||||
non-positive PID. It must never block and never take a lock.
|
||||
|
||||
In `lock.go`:
|
||||
|
||||
- Open each sidecar with platform no-follow semantics and reject a symlink,
|
||||
reparse point, or non-regular file before returning the handle. The opened
|
||||
handle, rather than a second path lookup, is the regular-file proof used for
|
||||
the later metadata write.
|
||||
- After `lockFile(file)` succeeds in `Acquire`, call `writeOwner(file)` and
|
||||
discard the error. A metadata write failure must not fail acquisition; the
|
||||
process owns the lock either way.
|
||||
- On the `errors.Is(err, ErrConflict)` branch, call `readOwner(target.LockPath)`
|
||||
and put the result on a new `Owner *OwnerRecord` field of `ConflictError`.
|
||||
- Suppress the detail when it cannot be trusted: if `proclive.Alive(rec.PID)`
|
||||
returns `(false, true)`, meaning positively gone, set `Owner` to `nil`. Keep it
|
||||
when liveness is unknown, `(false, false)`, which is what makes this useful on
|
||||
Windows.
|
||||
|
||||
Extend `ConflictError.Error()` rather than replacing it, so the existing sentence
|
||||
and the `use a separate KANDEV_HOME_DIR` guidance appended by `main.go` still
|
||||
read correctly:
|
||||
|
||||
```text
|
||||
home target "/Users/cfl12/.kandev" is already owned by another backend (pid 51229, /path/to/kandev, started 2026-08-19T09:14:35Z)
|
||||
```
|
||||
|
||||
With no trustworthy owner, the message must be byte-identical to today's.
|
||||
|
||||
Omit an empty executable from the parenthetical instead of rendering an empty
|
||||
string.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A second `Acquire` against a held lock returns a `*ConflictError` whose `Owner`
|
||||
carries the first process's PID and executable, and whose `Error()` includes
|
||||
them.
|
||||
- Empty, unparseable, or known-dead metadata produces exactly today's message.
|
||||
- A failed metadata write still yields a successful `Acquire`.
|
||||
- A symlinked lock path is rejected without modifying its target.
|
||||
- Owner metadata remains readable while the lock is held on Windows.
|
||||
- No code path reads `OwnerRecord` to decide whether a lock may be acquired.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/backendapp/...
|
||||
```
|
||||
|
||||
```bash
|
||||
cd apps/backend && gofmt -l internal/backendapp && golangci-lint run ./internal/backendapp/... --new-from-rev=origin/main --timeout=5m
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/backendapp/ownershiplock/owner.go` (new)
|
||||
- `apps/backend/internal/backendapp/ownershiplock/owner_test.go` (new)
|
||||
- `apps/backend/internal/backendapp/ownershiplock/lock.go`
|
||||
- `apps/backend/internal/backendapp/ownershiplock/lock_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 (`proclive.Alive`).
|
||||
|
||||
## Tests to write
|
||||
|
||||
In `lock_test.go`, extending the existing conflict coverage:
|
||||
|
||||
- Acquire a lock, attempt a second acquire on the same target, assert the
|
||||
returned `*ConflictError` has a non-nil `Owner` with this process's PID and
|
||||
that `Error()` contains both the path and the PID.
|
||||
- Table-driven fallback: pre-write lock-file contents of `""`, `"not json"`,
|
||||
`"{}"`, and a valid record whose PID an injected probe reports as
|
||||
`(false, true)`; assert `Owner` is nil and the message equals the pre-change
|
||||
wording in every case.
|
||||
- Unknown liveness, `(false, false)`, keeps the owner detail.
|
||||
- Metadata write failure still returns a usable owner from `Acquire`.
|
||||
- Create a symlinked sidecar and assert opening it fails without changing the
|
||||
target bytes.
|
||||
|
||||
In `owner_test.go`:
|
||||
|
||||
- Round-trip `writeOwner` then `readOwner`.
|
||||
- Truncation: pre-fill the file with a longer valid record, write a shorter one,
|
||||
read back and assert no trailing bytes survive.
|
||||
- Hold the OS lock and read owner metadata through a second handle.
|
||||
- `readOwner` on a missing file, a directory, and a non-positive PID returns
|
||||
`nil` without error.
|
||||
|
||||
Inject the liveness probe through a package-level `var` so tests do not depend on
|
||||
real PID lifetimes. `internal/backendapp/main_test.go`'s existing
|
||||
`TestBackendStartupConflictStopsBeforeSharedStateInitialization` must pass
|
||||
unchanged; it is the regression guard that this task did not disturb the
|
||||
fail-closed startup ordering.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the new files, the changed `ConflictError` shape, exact commands with
|
||||
outcomes, a sample of the enriched message, and explicit confirmation that no
|
||||
acquisition decision reads owner metadata. Update this file's `status` and
|
||||
`## Results`, and the matching checkbox in `plan.md`.
|
||||
|
||||
## Results
|
||||
|
||||
- Added advisory `OwnerRecord` JSON metadata with PID, executable, and RFC3339Nano
|
||||
start time; acquisition ignores metadata write failures.
|
||||
- Enriched `ConflictError` with owner details only when metadata is parseable and
|
||||
liveness is not positively known to be dead. Path-only wording remains the
|
||||
fallback.
|
||||
- Added regression coverage for owner rendering, truncation, invalid/dead
|
||||
metadata, unknown liveness, metadata-write failure, and existing startup
|
||||
conflict behavior.
|
||||
- `cd apps/backend && go test -run 'TestOwnershipLockConflict|TestOwnershipLockMetadataWriteFailure' ./internal/backendapp/ownershiplock/...`
|
||||
passed.
|
||||
- `cd apps/backend && go test ./internal/backendapp/...` passed for backendapp
|
||||
and ownershiplock.
|
||||
- `gofmt -w` completed for all changed ownership-lock Go files.
|
||||
- Cleanup: all in-process owners are closed with test cleanup or explicit close;
|
||||
no lock handles remain.
|
||||
- Security/trust and external side-effect boundaries: owner metadata never
|
||||
participates in acquisition, staleness, or takeover decisions; the OS lock
|
||||
remains authoritative.
|
||||
- Fixup hardening: platform no-follow sidecar opens reject symlinks/reparse
|
||||
points before metadata writes, and the reserved lock byte keeps Windows
|
||||
conflict diagnostics readable while the lock is held.
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
status: shipped
|
||||
created: 2026-06-23
|
||||
updated: 2026-08-11
|
||||
updated: 2026-08-19
|
||||
owner: tbd
|
||||
---
|
||||
|
||||
@@ -219,6 +219,17 @@ display before being shown.
|
||||
rejected without stopping the running application.
|
||||
- A missing packaged launcher/helper, backend bind failure, failed authenticated health check, or
|
||||
startup timeout produces a visible startup error and terminates the owned process tree.
|
||||
- On platforms with a reliable process-liveness probe, if the shell process ends without running
|
||||
its own shutdown path, because it crashed, was force quit, or was killed by the operating system,
|
||||
the owned launcher and backend must still exit on their own. Shell-initiated cleanup covers quit,
|
||||
startup failure, and update restart, but it cannot cover its own abnormal termination, so the
|
||||
launcher is additionally bound to the lifetime of the shell that spawned it. The shell publishes
|
||||
its process identity to the launcher at spawn time, and the launcher stops its supervised tree
|
||||
once that process is positively known to be gone. This is a hard requirement rather than
|
||||
best-effort cleanup on those platforms: a launcher that outlives its shell keeps the Kandev
|
||||
home's exclusive runtime-state lock and blocks every later desktop and CLI start until the
|
||||
operator finds and kills it by hand. Platforms without a reliable probe retain their existing
|
||||
normal-shutdown guarantee and do not infer parent death from an unknown liveness result.
|
||||
- A GUI environment that cannot find an agent CLI surfaces the existing setup-health guidance.
|
||||
- If backend shutdown fails during update restart, installation does not force an unclean restart;
|
||||
the user sees an actionable error and can quit normally.
|
||||
@@ -261,6 +272,11 @@ display before being shown.
|
||||
Settings page opens in the main window.
|
||||
- **GIVEN** the user clicks the macOS red close control, **WHEN** shutdown completes, **THEN** both
|
||||
the app and its owned backend exit.
|
||||
- **GIVEN** a desktop platform with a reliable process-liveness probe and a shell that ends without
|
||||
running its shutdown path, **WHEN** the launcher observes that the shell process it was spawned
|
||||
by is positively gone, **THEN** the launcher shuts down its backend, releases the Kandev home
|
||||
runtime-state lock, and exits, so the next desktop or CLI start acquires ownership without the
|
||||
operator killing a leftover process.
|
||||
- **GIVEN** a signed newer desktop version exists, **WHEN** an automatic check finds it, **THEN**
|
||||
Kandev prompts in the existing Updates page before any download, install, or restart.
|
||||
- **GIVEN** a Linux AppImage installation, **WHEN** the user confirms a valid update, **THEN** the
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
status: building
|
||||
created: 2026-08-07
|
||||
updated: 2026-08-11
|
||||
updated: 2026-08-19
|
||||
---
|
||||
|
||||
# Port collision and backend ownership safety
|
||||
@@ -130,6 +130,26 @@ database path and tells the operator to use a separate `KANDEV_HOME_DIR` for an
|
||||
instance. The rejected process must not initialize file logging, create a database backup, apply a
|
||||
migration, reconcile a session, launch agentctl, or start an HTTP server.
|
||||
|
||||
That message also names the current owner when the owner is knowable. After a backend acquires a
|
||||
lock it records advisory owner metadata, its process ID, executable path, and start time, into the
|
||||
lock file it already holds, replacing any previous content. A process rejected for a conflict reads
|
||||
that metadata and includes it in its stderr message, so an operator who has no leftover terminal can
|
||||
identify the process to stop instead of being told only that the home is busy.
|
||||
|
||||
This metadata is diagnostics, not ownership. The operating-system lock remains the sole proof: the
|
||||
metadata is never consulted to decide whether a lock is held, whether it is stale, or whether it may
|
||||
be taken over, and acquisition never waits on it. Failing to write it does not fail acquisition, and
|
||||
absent, empty, or unparseable metadata falls back to the message that names only the path. Because a
|
||||
crashed owner leaves its last-written metadata in place, a reader that can positively determine the
|
||||
recorded process is no longer running omits the owner detail rather than naming a dead process; a
|
||||
platform that cannot determine liveness reports the recorded values as-is.
|
||||
|
||||
The lock sidecar is opened with platform no-follow semantics and must resolve to a regular file before
|
||||
the backend can write metadata. A symlink or platform reparse point at the lock path is rejected
|
||||
without modifying its target. The first byte of the sidecar is reserved for the Windows lock range,
|
||||
so a conflicting process can read the advisory metadata through a separate handle while the lock is
|
||||
held.
|
||||
|
||||
The backend fails closed when it cannot create, open, or acquire a required lock. Launcher port
|
||||
preflight and health-token ownership remain useful readiness checks, but they are not persistent
|
||||
state ownership checks.
|
||||
@@ -201,13 +221,25 @@ not part of this contract.
|
||||
against that home, then the direct command fails without changing task or session state.
|
||||
5. Given two distinct homes, databases, and ports, when two local backends start, then both can run
|
||||
as independent instances.
|
||||
6. Given a backend owns a Kandev home and has recorded its owner metadata, when a second backend
|
||||
starts against the same home, then the rejection names the owning process ID and executable in
|
||||
addition to the conflicting home path.
|
||||
7. Given a lock file still holds metadata from an owner that has since died, when a second backend
|
||||
is rejected by whichever process holds the lock now, then the message omits owner detail it
|
||||
cannot confirm and still names the conflicting home path.
|
||||
8. Given a backend cannot write its owner metadata, when it has already acquired the lock, then it
|
||||
starts normally and a later conflicting process falls back to the path-only message.
|
||||
9. Given a lock sidecar path is a symlink or platform reparse point, when the backend starts, then
|
||||
it rejects the lock before writing and leaves the link target unchanged.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing default ports, fallback ranges, or automatic-port selection policy.
|
||||
- Choosing a different port when the user explicitly requested one.
|
||||
- PID or process-tree identity matching; the launcher wrapper chain makes that unreliable for
|
||||
dev mode.
|
||||
- PID or process-tree identity matching as an ownership, staleness, or takeover decision; the
|
||||
launcher wrapper chain makes that unreliable for dev mode, and a recorded PID can be stale or
|
||||
reused. Advisory owner metadata written into the lock file is diagnostic text only and is
|
||||
explicitly not such a decision input.
|
||||
- Changing the health response body, the backend health status semantics, or the desktop
|
||||
WebView flow.
|
||||
- Renaming KANDEV_DESKTOP_HEALTH_TOKEN to a neutral variable; that would be a separate contract
|
||||
|
||||
Reference in New Issue
Block a user