Fix upgrade migration, config validation, and CLI timeouts - #482
Merged
Merged
Conversation
…ed probe The probe ran pilot-daemon -help once; ETXTBSY (binary still open for writing: an updater swapping it, or a concurrently forked process holding the write fd) made it report the daemon's flags as unknown, and the empty result was cached. Seen as TestDaemonFlagUsageParsesUsage failing in CI under parallel tests. Retry up to 5x with 50 ms spacing within the probe timeout, and only cache successful probes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Legacy macOS upgrades can leave the old daemon loaded and schedule a duplicate at login. Config writes accept invalid values, and send-message callers cannot bound connection/handshake time. This change migrates launchd labels, validates config writes, honors configured encryption, and adds a total
--timeoutbudget.--timeoutcovers lookup, handshake, dial, ACK and reply;--waitretains its documented reply-only behavior. Timeout errors note that delivery may already have occurred.Fixes #475. Fixes #476. Fixes #477. Fixes #480. Addresses the CLI portions of #481.
The launchd log claim in #481 is already addressed by #464: daemon log rotation discovers redirected stdout/stderr and defaults to a 50 MB cap. A 5.3 MB log is below that cap. Empty app lists already exited zero; this PR also makes their output consistent. The repeated macOS resource-limit warning is fixed separately in app-store. #479's first-contact changes remain in existing PR #474; its focused regression tests pass and are not duplicated here.
Validation: new issue regression tests (including stalled lookup, handshake, dial, ACK and reply); full cmd, daemon and logcap suites;
GOWORK=off go test -parallel 4 -count=1 ./tests/(passed, 351 s);GOWORK=off go build ./...; go vet; installer shell syntax; all commit hooks with GOWORK=off. Tests use isolated homes and fake launchctl, not the host's services.Merge dependency: canonical installer PR pilot-protocol/release#50 must land first; both scripts are byte-identical. The core installer-parity check remains correctly failing until then. The PR-event architecture run and Linux/macOS tests pass; an earlier push-event run on the same commit hit the existing daemon-help subprocess timing failure. App-store warning fix: pilot-protocol/app-store#41.