Repository navigation
appstore: install waits for the app's first start and reports it - #497
Merged
Merged
Conversation
TeoSlayer
force-pushed
the
fix/appstore-install-health
branch
from
October 7, 2026 13:09
a4fbf20 to
3c19862
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| fatalHint("invalid_argument", "usage: --wait <duration>, e.g. --wait 60s (0 is --no-wait)", "--wait needs a value") | ||
| return | ||
| } | ||
| d, err := time.ParseDuration(args[i+1]) |
| } | ||
| d, err := time.ParseDuration(args[i+1]) | ||
| if err != nil || d < 0 { | ||
| fatalHint("invalid_argument", "usage: --wait <duration>, e.g. --wait 60s (0 is --no-wait)", "invalid --wait %q", args[i+1]) |
install only writes files; the daemon's supervisor starts the app on its next rescan. So install exited 0 for an app that then exited at every start (io.pilot.sqlite on an image without tar), and the app was suspended about half a minute later with nothing in the install output. With a daemon listening, install now watches the app dir (supervisor.log, .suspended, app.sock) for up to 20s after the swap and reports: - started: a spawn since the install and the socket is there; - failed: the supervisor suspended the app, or it exited at least twice with no socket (three times when an install was replaced, since the instance the supervisor stops logs an exit too). install prints its report and then exits non-zero (app_start_failed) with the last supervisor log line and pointers to `appstore audit` and the daemon log; - starting: neither by the deadline. Exit 0 with a note, so an app that takes longer than the wait to open its socket is not a failed install. Waiting for .suspended alone would not do: the supervisor suspends after more than 5 exits in a minute, 31s at the earliest. No wait with --no-wait, when no daemon is listening, or for a reinstall the supervisor does not act on (same version and binary, or an older version). `appstore upgrade` passes --no-wait: it would otherwise report a failed start as "was not upgraded", and the hourly `upgrade --all` would spend the wait on every app. The JSON report gains start_state, start_exits, start_detail and start_waited_ms; existing fields are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review fixes for the wait on the app's first start. The replaced instance's events no longer count. supervisor.log is carried into the new install and the supervisor keeps running the old instance until its next rescan, so lines after the swap can still be the old instance's: the end of its crash loop (exits, a suspend and its .suspended marker) made a good upgrade report failed and exit 1, and the exit it logs when the rescan stops it showed up as start_exits: 1 on a clean upgrade. Counting now starts at the new instance's supervise-start or spawn, recognised by the installed manifest's binary sha256 they carry; an exit counts only when it ends one of the new instance's start attempts, and a suspend only when it follows one of its exits. The .suspended marker is no longer read: it names no instance, and the log has the suspend first. A rotated supervisor.log.1 written during the wait is read too. Versions are ordered as the supervisor orders them (a release above its prereleases), not with semverCompare, which ignores prereleases: --force from 1.0.0 to 1.0.0-beta.1 waited 20s for a start the supervisor refuses. A downgrade-refused line for the installed version is now a final outcome: failed, with the supervisor's reason and how to get the version running. A downgrade that is not waited for says the daemon starts it only when it restarts, instead of "picked up within ~30s". After a spawn-fail the failure reason is reported, not the exit -1 that follows it. A wait that ends before the daemon picked the app up prints one note instead of two that contradict each other. --wait <duration> changes the 20s (0 is --no-wait). --no-wait and --wait are in the main help, the JSON command catalogue and docs/cli-reference.md. Tests: the three install tests in zz_appstore_cmds_test.go dialled the daemon at /tmp/pilot.sock and waited 20s each when one was running; they now use isolateAppStoreTest. The fake-supervisor helpers report with t.Error, since they run on goroutines. New tests cover the upgrade log sequences, the refusal and the prerelease order, with the fake and the real supervisor; all of them fail on the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review fix. With a binary identical across versions (a manifest-only bump) the old instance's restarts after the swap log the new manifest's sha, so they were counted, and two old crashes could report the new install as failed. The counts now restart at the new instance's supervise-start. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TeoSlayer
force-pushed
the
fix/appstore-install-health
branch
from
October 7, 2026 14:11
60e2690 to
63c982a
Compare
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.
Pull Request
Summary
pilotctl appstore installreturned success as soon as the files were in place. An app that then crashed at every start (seen with an image missingtar) was only suspended about 31 seconds later, and install had long since exited 0 with no hint.Changes
With a daemon listening, install waits up to 20s after the swap and reads the app's
supervisor.log,.suspendedand socket:app_start_failed) with the last supervisor log line and pointers toappstore audit, the daemon log,restartanduninstall.No wait with
--no-wait, with no daemon reachable, for a--forcereinstall of the same version and binary, or for a downgrade the supervisor will refuse. JSON gainsstart_state,start_exits,start_detail,start_waited_ms; existing fields are unchanged.Also:
appstore upgradepasses--no-wait: otherwise a failed start would be reported as "was not upgraded", which is false, and the hourlyupgrade --allwould wait on every app.Behaviour change to note: scripts that treat install as file placement will see a new non-zero exit when the app crashes at start.
Test Plan
go build ./...,go vet ./..., unit suite withGOWORK=off-short, so CI's unit run does not execute them — run without-short, they passChecklist
go.mod/go.sumunchanged🤖 Generated with Claude Code