Skip to content

Commit f25f77b

Browse files
committed
fix(pilotctl,install,daemon-tests): rc5 UX gaps + flaky panic-survival tests
Three UX gaps from the rc4 clean-install flow: 1. macOS install.sh installs a launchd plist (KeepAlive=true), so the daemon lifecycle is owned by launchd rather than by the PID file pilotctl writes when it forks. `pilotctl daemon stop` returned "daemon socket is active but PID file is missing" and refused to act; SIGTERM would have respawned anyway via KeepAlive. pilotctl now detects `~/Library/LaunchAgents/com.vulturelabs.pilot -daemon.plist` and routes start/stop through `launchctl bootstrap gui/<UID> <plist>` / `launchctl bootout gui/<UID>/<label>`. The pilotctl-fork path stays as the fallback for Linux + manual setups. 2. `pilotctl send-message <peer> --data ... --wait` would ACK the outbound but the responder's reply SYN was rejected by the local trust gate (`SYN rejected: untrusted source`). --wait then timed out with no actionable signal. The compiled-in trustedagents allowlist protects this for the agents it covers, but stale entries (a public agent redeployed under a new node ID) and any public peer not in the list fall through to the "pub=true → return silently" path. maybeAutoHandshake's Branch 3 (public peer, not in compiled list) now runs a best-effort handshake + WaitForTrust(5s) before returning. Public auto-approve peers finalise mutual trust in ~700-2400 ms; replies pass the local trust gate naturally afterward. 3. install.sh `PILOT_RC=1` hits api.github.com/repos/.../releases. When the unauthenticated 60/hr limit is exhausted the API returns 403, the existing code captured TAG="" silently, and the script fell through to source-build of `main` HEAD with no -ldflags version stamp — the user thought they got the latest RC but got an unstamped "dev" binary. install.sh now: - accepts `PILOT_RELEASE_TAG=vX.Y.Z-rcN` as an explicit override (skips the API entirely); - separates the API body from its HTTP status code so a 403 is detected explicitly + reported, with a clear suggestion to set PILOT_RELEASE_TAG or retry in ~1h, rather than silently degrading to a source build. Also: pkg/daemon/zz_panic_survival_test.go — the 10 TestL*PanicSurvival tests all called t.Parallel() while sharing the package-global recoveredPanicCount counter. resetRecoveredPanicCountForTest() in one test could wipe the increment of another's recoverLayer mid-assertion, producing a flaky FAIL ("did not catch nil-packet panic") even though the panic WAS recovered (CI logs showed recovered_total=1 immediately before the FAIL). Drop t.Parallel() from all ten; each test is <50ms so serialisation has no measurable cost. Verified live on a clean install: - daemon start/stop round-trip via launchctl, no PID-file error; - untrust 179172 + send-message list-agents --wait now succeeds end-to-end with the reply (~2.2 KB directory dump) inline; - `go test -count=10 -run TestL.*PanicSurvival ./pkg/daemon/` passes.
1 parent 9a44092 commit f25f77b

4 files changed

Lines changed: 185 additions & 22 deletions

File tree

cmd/pilotctl/main.go

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"os"
1515
"os/exec"
1616
"path/filepath"
17+
"runtime"
1718
"sort"
1819
"strconv"
1920
"strings"
@@ -541,7 +542,28 @@ func maybeAutoHandshake(d *driver.Driver, addr protocol.Addr, skip bool) {
541542
fmt.Sprintf("run: pilotctl handshake %s", addr),
542543
"refusing tunnel to private node %s without trust", addr)
543544
}
544-
// Public peer, no trust needed — proceed.
545+
// Public peer — best-effort handshake so replies survive our local
546+
// trust gate. Without this, a request/reply pattern (e.g. send-message
547+
// --wait) gets its ACK back but the responder's reply SYN is silently
548+
// rejected by our daemon (daemon.go SYN-trust-gate). The peer is
549+
// public and the embedded trustedagents allowlist may not cover every
550+
// public hostname (entries can become stale when a public agent is
551+
// re-deployed under a new node ID), so we treat any registry-resolved
552+
// public peer the same as a Trusted Agent. Auto-approve peers (most
553+
// public agents are) finalise mutual trust in ~700-2400 ms; non-auto-
554+
// approve peers still see the request in pending and the tunnel
555+
// proceeds best-effort.
556+
if _, err := d.Handshake(addr.Node, "auto-handshake: public peer"); err != nil {
557+
// Handshake send failure is non-fatal — fall through to send.
558+
return
559+
}
560+
if resp, err := d.WaitForTrust(addr.Node, 5000); err == nil {
561+
if trusted, _ := resp["trusted"].(bool); trusted {
562+
if !jsonOutput {
563+
fmt.Fprintf(os.Stderr, "auto-handshake established trust with public peer %s\n", addr)
564+
}
565+
}
566+
}
545567
}
546568

547569
func parseAddrOrHostname(d *driver.Driver, arg string) (protocol.Addr, error) {
@@ -2111,9 +2133,94 @@ func buildDaemonArgs(args []string) (daemonArgs []string, socketPath string) {
21112133
return daemonArgs, socketPath
21122134
}
21132135

2136+
// launchdAgentLabels enumerates known launchd labels for the daemon.
2137+
// install.sh renamed `com.vulturelabs.pilot-daemon` to
2138+
// `network.pilotprotocol.pilot-daemon`; the uninstall path handles both
2139+
// for backward compat, and so does pilotctl. The first match wins.
2140+
var launchdAgentLabels = []string{
2141+
"network.pilotprotocol.pilot-daemon",
2142+
"com.vulturelabs.pilot-daemon",
2143+
}
2144+
2145+
// launchdAgentPlist returns the path to the per-user launchd plist that
2146+
// install.sh creates on macOS, if it exists; otherwise "". When non-empty
2147+
// the daemon's lifecycle is owned by launchd (not by the PID file pilotctl
2148+
// writes when it forks the daemon itself), and stop/start must go through
2149+
// launchctl so KeepAlive doesn't immediately respawn what we kill.
2150+
// Returns the plist path AND the label embedded in it.
2151+
func launchdAgentPlist() (path, label string) {
2152+
if runtime.GOOS != "darwin" {
2153+
return "", ""
2154+
}
2155+
home, err := os.UserHomeDir()
2156+
if err != nil {
2157+
return "", ""
2158+
}
2159+
for _, lbl := range launchdAgentLabels {
2160+
p := filepath.Join(home, "Library", "LaunchAgents", lbl+".plist")
2161+
if _, err := os.Stat(p); err == nil {
2162+
return p, lbl
2163+
}
2164+
}
2165+
return "", ""
2166+
}
2167+
2168+
// launchdAgentDomainTarget returns the gui/<UID>/<label> target used by
2169+
// `launchctl bootout` / `launchctl kickstart`.
2170+
func launchdAgentDomainTarget(label string) string {
2171+
return fmt.Sprintf("gui/%d/%s", os.Getuid(), label)
2172+
}
2173+
2174+
// launchdAgentLoaded reports whether the launchd label is currently
2175+
// loaded in the user's gui domain (i.e. plist registered, KeepAlive etc.
2176+
// are active even if the underlying process is briefly between restarts).
2177+
func launchdAgentLoaded(label string) bool {
2178+
if label == "" {
2179+
return false
2180+
}
2181+
out, err := exec.Command("launchctl", "list", label).Output()
2182+
if err != nil {
2183+
return false
2184+
}
2185+
return len(out) > 0
2186+
}
2187+
21142188
func cmdDaemonStart(args []string) {
21152189
flags, _ := parseFlags(args)
21162190

2191+
// macOS install.sh installs a launchd plist. When present, route start
2192+
// through launchctl so the agent is registered and KeepAlive supervises
2193+
// the process; otherwise `pilotctl daemon stop` would have nothing to
2194+
// stop (KeepAlive immediately respawns) and the user sees flapping.
2195+
if plist, label := launchdAgentPlist(); plist != "" {
2196+
if launchdAgentLoaded(label) {
2197+
fatalHint("already_exists",
2198+
"stop it first with: pilotctl daemon stop",
2199+
"daemon is already running (launchd agent %s loaded)", label)
2200+
}
2201+
if err := exec.Command("launchctl", "bootstrap", fmt.Sprintf("gui/%d", os.Getuid()), plist).Run(); err != nil {
2202+
fatalCode("internal", "launchctl bootstrap: %v", err)
2203+
}
2204+
// Poll socket until daemon is responsive.
2205+
waitDeadline := time.Now().Add(10 * time.Second)
2206+
for time.Now().Before(waitDeadline) {
2207+
if d, err := driver.Connect(getSocket()); err == nil {
2208+
if _, err := d.Info(); err == nil {
2209+
d.Close()
2210+
if jsonOutput {
2211+
outputOK(map[string]interface{}{"managed_by": "launchd", "label": label})
2212+
} else {
2213+
fmt.Printf("daemon started via launchd (label %s)\n Socket: %s\n", label, getSocket())
2214+
}
2215+
return
2216+
}
2217+
d.Close()
2218+
}
2219+
time.Sleep(200 * time.Millisecond)
2220+
}
2221+
fatalCode("timeout", "launchd loaded the agent but the socket did not become ready within 10s")
2222+
}
2223+
21172224
// Check if already running
21182225
if pid := readPID(); pid > 0 {
21192226
if processExists(pid) {
@@ -2254,6 +2361,32 @@ func cmdDaemonStart(args []string) {
22542361
}
22552362

22562363
func cmdDaemonStop() {
2364+
// macOS install.sh installs a launchd plist with KeepAlive=true. When
2365+
// we own it via launchd, SIGTERM to the PID respawns the process
2366+
// immediately — the right way to stop is `launchctl bootout`. Detect
2367+
// this case before falling back to PID-file management.
2368+
if plist, label := launchdAgentPlist(); plist != "" && launchdAgentLoaded(label) {
2369+
if err := exec.Command("launchctl", "bootout", launchdAgentDomainTarget(label)).Run(); err != nil {
2370+
fatalCode("internal", "launchctl bootout: %v", err)
2371+
}
2372+
// Wait for the daemon socket to disappear (launchd reaps the
2373+
// process); the agent stays unloaded across reboots until the
2374+
// user calls `pilotctl daemon start` (bootstrap) again.
2375+
waitDeadline := time.Now().Add(10 * time.Second)
2376+
for time.Now().Before(waitDeadline) {
2377+
if !launchdAgentLoaded(label) {
2378+
if jsonOutput {
2379+
outputOK(map[string]interface{}{"managed_by": "launchd", "label": label})
2380+
} else {
2381+
fmt.Printf("daemon stopped (launchd agent %s unloaded)\n", label)
2382+
}
2383+
return
2384+
}
2385+
time.Sleep(200 * time.Millisecond)
2386+
}
2387+
fatalCode("timeout", "launchctl bootout returned but agent did not unload within 10s")
2388+
}
2389+
22572390
pid := readPID()
22582391
if pid <= 0 {
22592392
// Try socket

pkg/daemon/zz_panic_survival_test.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
// length-check error paths; the boundary is defense-in-depth for any
1919
// future code change that introduces a slice OOB.
2020
func TestL1UnmarshalPanicSurvival(t *testing.T) {
21-
t.Parallel()
2221
cases := [][]byte{
2322
nil, // nil slice
2423
{}, // empty
@@ -38,7 +37,6 @@ func TestL1UnmarshalPanicSurvival(t *testing.T) {
3837
// known panic-free path returns "payload too large" for oversized
3938
// payloads; the boundary protects against future panic introductions.
4039
func TestL1MarshalPanicSurvival(t *testing.T) {
41-
t.Parallel()
4240
huge := make([]byte, 0xFFFF+1)
4341
pkt := &protocol.Packet{
4442
Version: protocol.Version,
@@ -60,7 +58,6 @@ func TestL1MarshalPanicSurvival(t *testing.T) {
6058
// burst. Real edge case: junk magic bytes, truncated headers, a too-
6159
// short PILS frame.
6260
func TestL2ReadLoopPanicSurvival(t *testing.T) {
63-
t.Parallel()
6461
tm := NewTunnelManager()
6562
if err := tm.Listen("127.0.0.1:0"); err != nil {
6663
t.Fatalf("Listen: %v", err)
@@ -126,7 +123,6 @@ func TestL2ReadLoopPanicSurvival(t *testing.T) {
126123
// deliver). The boundary keeps the function alive — process must not
127124
// crash. Real edge cases used.
128125
func TestL4BeaconMessagePanicSurvival(t *testing.T) {
129-
t.Parallel()
130126
tm := NewTunnelManager()
131127
defer tm.Close()
132128
from := &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 1}
@@ -151,7 +147,6 @@ func TestL4BeaconMessagePanicSurvival(t *testing.T) {
151147
// on a synthetic Daemon. With nil registry conn the function returns
152148
// cleanly; the boundary protects against future panic introductions.
153149
func TestL4BeaconRefreshTickPanicSurvival(t *testing.T) {
154-
t.Parallel()
155150
d := &Daemon{}
156151
d.beaconRefreshTick(true)
157152
d.beaconRefreshTick(false)
@@ -170,7 +165,6 @@ func TestL4BeaconRefreshTickPanicSurvival(t *testing.T) {
170165
// TestL5HandleAuthKeyExchangePanicSurvival drives handleAuthKeyExchange
171166
// with malformed inputs (too-short, all-zero). Real edge cases.
172167
func TestL5HandleAuthKeyExchangePanicSurvival(t *testing.T) {
173-
t.Parallel()
174168
tm := NewTunnelManager()
175169
if err := tm.EnableEncryption(); err != nil {
176170
t.Fatalf("EnableEncryption: %v", err)
@@ -184,7 +178,6 @@ func TestL5HandleAuthKeyExchangePanicSurvival(t *testing.T) {
184178
// TestL5HandleKeyExchangePanicSurvival drives handleKeyExchange with
185179
// malformed inputs.
186180
func TestL5HandleKeyExchangePanicSurvival(t *testing.T) {
187-
t.Parallel()
188181
tm := NewTunnelManager()
189182
if err := tm.EnableEncryption(); err != nil {
190183
t.Fatalf("EnableEncryption: %v", err)
@@ -200,7 +193,6 @@ func TestL5HandleKeyExchangePanicSurvival(t *testing.T) {
200193
// triggers a rate-limited rekey request and returns. The boundary
201194
// keeps the daemon alive across a burst.
202195
func TestL6HandleEncryptedPanicSurvival(t *testing.T) {
203-
t.Parallel()
204196
tm := NewTunnelManager()
205197
if err := tm.EnableEncryption(); err != nil {
206198
t.Fatalf("EnableEncryption: %v", err)
@@ -226,7 +218,6 @@ func TestL6HandleEncryptedPanicSurvival(t *testing.T) {
226218
// Uses test-only injection: replacing RetxSend with a panicking
227219
// closure is the cleanest way to hit a real code path with a panic.
228220
func TestL7RetxLoopPanicSurvival(t *testing.T) {
229-
t.Parallel()
230221
d := New(Config{
231222
ListenAddr: "127.0.0.1:0",
232223
IdentityPath: t.TempDir() + "/identity.json",
@@ -285,7 +276,6 @@ func TestL7RetxLoopPanicSurvival(t *testing.T) {
285276
// a nil packet, which would nil-deref on pkt.Protocol. The boundary
286277
// must catch it.
287278
func TestL7RoutePacketPanicSurvival(t *testing.T) {
288-
t.Parallel()
289279
d := New(Config{
290280
ListenAddr: "127.0.0.1:0",
291281
IdentityPath: t.TempDir() + "/identity.json",
@@ -310,7 +300,6 @@ func TestL7RoutePacketPanicSurvival(t *testing.T) {
310300
// no real unrecovered panic surface exists today. This test guarantees
311301
// the L9 boundary mechanism would catch one if introduced.
312302
func TestL9IPCHandlerPanicSurvival(t *testing.T) {
313-
t.Parallel()
314303
d := New(Config{
315304
ListenAddr: "127.0.0.1:0",
316305
IdentityPath: t.TempDir() + "/identity.json",

tests/zz_eventstream_broker_parity_test.go

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,29 +136,49 @@ func TestBrokerParityNormal(t *testing.T) {
136136
}
137137

138138
// Receive must preserve topic + payload bytes exactly.
139+
// waitForSubscription's probes ride the same topic, so the broker may
140+
// deliver a left-over probe before the real event arrives. Loop on Recv
141+
// and skip probes; bail only on the real payload (or timeout). The
142+
// previous single-Recv form was flaky on macOS CI where the probe
143+
// reached the subscriber before the real publish.
139144
deadline := time.After(5 * time.Second)
140145
type res struct {
141146
evt *eventstream.Event
142147
err error
143148
}
144-
ch := make(chan res, 1)
149+
stopRecv := make(chan struct{})
150+
recvCh := make(chan res, 4)
145151
go func() {
146-
evt, err := sub.Recv()
147-
ch <- res{evt, err}
152+
for {
153+
evt, err := sub.Recv()
154+
select {
155+
case recvCh <- res{evt, err}:
156+
case <-stopRecv:
157+
return
158+
}
159+
if err != nil {
160+
return
161+
}
162+
}
148163
}()
164+
defer close(stopRecv)
149165

150166
// Re-publish on a ticker in case the probe drained the real one.
151167
tick := time.NewTicker(100 * time.Millisecond)
152168
defer tick.Stop()
153169
for {
154170
select {
155-
case r := <-ch:
171+
case r := <-recvCh:
156172
if r.err != nil {
157173
t.Fatalf("recv: %v", r.err)
158174
}
159175
if r.evt.Topic != "parity.normal" {
160176
t.Fatalf("topic = %q, want %q", r.evt.Topic, "parity.normal")
161177
}
178+
if string(r.evt.Payload) == "__probe__" {
179+
// Subscription-readiness probe — skip and keep reading.
180+
continue
181+
}
162182
if string(r.evt.Payload) != string(want) {
163183
t.Fatalf("payload mismatch: got %q want %q", r.evt.Payload, want)
164184
}

web/public/install.sh

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,33 @@ trap 'rm -rf "$TMPDIR"' EXIT
194194

195195
ARCHIVE="pilot-${OS}-${ARCH}.tar.gz"
196196

197-
# Resolve the latest release tag.
198-
# - Default path uses the unauthenticated /releases/latest/download/ redirect,
199-
# which is not subject to the 60/hr api.github.com rate limit.
200-
# - PILOT_RC=1 still hits the API because pre-releases need the listing endpoint.
201-
if [ "${PILOT_RC:-}" = "1" ]; then
202-
TAG=$(curl -fsSL "https://api.github.com/repos/${REPO}/releases" 2>/dev/null | grep '"tag_name"' | head -1 | cut -d'"' -f4 || true)
197+
# Resolve the release tag.
198+
# - PILOT_RELEASE_TAG=v1.2.3 : explicit override, no network round-trip.
199+
# - PILOT_RC=1 : list releases via api.github.com and pick the
200+
# newest (incl. pre-releases). 403 rate-limited
201+
# output is detected and reported instead of
202+
# silently falling through to source-build with
203+
# an unstamped version.
204+
# - default : follow the /releases/latest/download/ redirect,
205+
# which uses the unauthenticated CDN and is not
206+
# subject to the 60/hr api.github.com rate limit.
207+
if [ -n "${PILOT_RELEASE_TAG:-}" ]; then
208+
TAG="$PILOT_RELEASE_TAG"
209+
elif [ "${PILOT_RC:-}" = "1" ]; then
210+
API_BODY="$TMPDIR/releases.json"
211+
API_CODE=$(curl -sSL -o "$API_BODY" -w '%{http_code}' "https://api.github.com/repos/${REPO}/releases" 2>/dev/null || echo "000")
212+
if [ "$API_CODE" = "403" ]; then
213+
echo "Error: GitHub API rate-limited (403) while resolving the latest pre-release." >&2
214+
echo " Workarounds:" >&2
215+
echo " - retry in ~1 hour, OR" >&2
216+
echo " - pin the tag: PILOT_RELEASE_TAG=vX.Y.Z-rcN curl ... | sh" >&2
217+
echo " Refusing to silently source-build an unstamped binary." >&2
218+
exit 1
219+
fi
220+
if [ "$API_CODE" = "200" ]; then
221+
TAG=$(grep '"tag_name"' "$API_BODY" | head -1 | cut -d'"' -f4 || true)
222+
fi
223+
rm -f "$API_BODY"
203224
else
204225
TAG=$(curl -fsSI "https://github.com/${REPO}/releases/latest/download/${ARCHIVE}" 2>/dev/null \
205226
| grep -i '^location:' \

0 commit comments

Comments
 (0)