Skip to content

appstore: keep app state across install --force and upgrade - #467

Merged
TeoSlayer merged 4 commits into
mainfrom
fix/appstore-preserve-app-state
Sep 23, 2026
Merged

TeoSlayer merged 4 commits into
mainfrom
fix/appstore-preserve-app-state

Conversation

@TeoSlayer

@TeoSlayer TeoSlayer commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

pilotctl appstore install --force and appstore upgrade delete everything an app keeps in its own directory. The hourly updater runs upgrade --all, so the next wallet release in the catalogue would delete every installed wallet's EVM private key and payment database. This PR carries app state across the swap, keeps the replaced install as a backup and never deletes it outright. It also blocks catalogue releases of stateful apps until nodes run a pilotctl with this fix.

Problem (reliability sweep, item 1, P0; finding DOCS-01)

  • On main (20bd501), cmd/pilotctl/appstore.go:1270-1290 renames the live app dir to <id>.previous, renames staging into place, then runs _ = os.RemoveAll(previousDir) (line 1290). Nothing is copied forward.
  • cmd/pilotctl/appstore_update.go:241 runs every auto-upgrade through the same path (cmdAppStoreInstall([]string{o.ID, "--force"})). pilot-updater does this every hour (app auto-upgrade loop started interval=1h0m0s).
  • Apps keep state in that dir by design. The wallet has identity-evm.json, identity.json and data.db, and its manifest grants fs.write on $APP/identity-evm.json and $APP/data.db. smol has secrets.json. agentphone, bowmark and orthogonal each have identity.json.
  • A scratch-root reproduction (install --local, add identity-evm.json and data.db, reinstall with --local --force) deletes both files and leaves no .previous copy.
  • It already happened on one host. updater.log shows upgrading io.pilot.bowmark 0.1.0 → 1.0.1 at 2026-08-16 12:34:09, and bowmark's identity.json has mtime 12:34:10, so it was regenerated. Every other app kept its Aug 2-4 identity.
  • The heartbeat and app-store skill tell agents to run install <id> --force as the routine install step.
  • The new regression test fails on main: TestAppStoreForceReinstallKeepsAppState: after install --force: app state identity-evm.json was lost.

Also covered here: sweep item 16 (finding ci-07). wallet 0.3.3 and cosift 0.1.2 ship an arm64 Mach-O binary, and generallegal 0.1.0 ships an x86-64 ELF, all under a single legacy bundle_url. resolveBundle (appstore_catalogue.go:134-137) returns that URL unchanged. Main already refuses thin ELF/Mach-O mismatches (executable_platform.go, since #450), but universal Mach-O and PE images passed unchecked.

Changes

Install and upgrade keep state (cmd/pilotctl/appstore_state.go, appstore.go)

  • Every entry of the live dir that is not part of the bundle is carried into staging before the swap. Hard links are used, with a copy fallback. Not carried: manifest.json, install.json/.sh, .sideloaded, .suspended/.resume, .bundle-sha256, next-steps caches, the old binary and sockets/*.sock. manifest.json is written last, so the supervisor never adopts a half-filled staging dir.
  • The live dir stays at <id>.previous until the new dir verifies: exact manifest, pinned binary sha and carried state. Any failure restores it, discards staging and reports where the previous install actually is.
  • Writes the running app makes during the install are kept. After the swap, the old dir is walked again. Files replaced or created since the carry are linked in, and in-place rewrites of copied files are copied again. When the new install's copy also changed, the newer write wins. Volatile sidecars the app deleted (-journal, -wal, -shm, .lock, .pid, .tmp) are dropped, because a stale rollback journal would roll back a committed transaction.
  • The replaced install moves out of the install root to app-backups/<id>/<ts>-v<ver>/ beside it ($PILOT_APPSTORE_BACKUP_ROOT overrides). It moves out because the supervisor adopts any dir there that has a manifest. The old binary is stripped, and each backup gets .pilot-backup.json. The newest 3 upgrade backups and the newest 3 reinstall backups are kept. reset-state, incomplete, recovered and backups without metadata are never pruned, and every prune prints a note.
  • Backup fallbacks: a backup root on another filesystem is handled by copying the tree (EXDEV). If a location cannot be used, the backup goes to the default location, then to <install root>/.app-backups/<id>/, with a warning. Only if all of those fail is the dir parked as <id>.previous-<ts> with its manifest disabled. A configured backup root inside the install root is refused.
  • State that cannot be carried is handled without aborting. Read-only dirs (0555, like a Go module cache) carry. An entry this user can neither link nor read is moved in by rename after the swap. Anything left over stays in a pinned backup and is named in a warning and in state_not_carried. Any other carry error (disk full, I/O) aborts with nothing changed.
  • Crash recovery restores a lone <id>.previous and backs up a leftover one. It never deletes either, and it discards a leftover <id>.staging.
  • A per-app flock on <install root>/.<id>.lock serializes install, upgrade and uninstall of one app. A second one waits up to 5 minutes with a note, then fails with timeout.

CLI surface

  • install <id> on an installed app is a read-only no-op (exit 0) that points to upgrade. On main it failed with conflict. conflict now means that --version X or a local bundle names a different version without --force. A version the catalogue lacks fails with version_unavailable.
  • New --reset-state (implies --force) is the explicit destructive case. It prints a loud warning, does not carry state, and still keeps the backup.
  • upgrade --all traps fatal exits per app (fatal_trap.go), goes on to the next app and exits 1 at the end with upgrade_failed, naming the apps that failed. A single-app upgrade behaves as before.
  • Install JSON adds already_installed, hint, preserved_state, state_reset, state_not_carried, backup_dir and backup_warning. Uninstall notes remaining backups in every location and lists them in JSON as backups.

Platform check (cmd/pilotctl/appstore_platform.go)

  • Universal Mach-O and PE images are now checked too. The refusal names the app, version and host platform and says nothing was installed.

Catalogue lint (catalogue/lint, .github/workflows/catalogue-lint.yml, catalogue/stateful-apps.json)

  • Fails a PR that updates a stateful app, for a new version or a same-version republish. Stateful means listed in stateful-apps.json (wallet, smol, agentphone, bowmark, orthogonal), or any app whose old or new manifest grants fs.write or key.sign. A bundle that cannot be inspected counts as stateful. To override, add an approved_bumps entry or the PR label catalogue:stateful-bump-approved.
  • For each added or changed entry it downloads the bundle and checks the sha pin, manifest id and version, the binary pin, and that the binary runs on the platform it is published for. Documented in catalogue/README.md.

CHANGELOG: an entry under [Unreleased] / Fixed.

Review outcome

An independent review found 6 issues on the first commit (dc0e651). All 6 are fixed in 5a051e9:

  • F1: carry failed closed on read-only or unreadable state, and upgrade --all stopped at the first failed app.
  • F2: backup rotation could remove the only copy of state.
  • F3: concurrent installs of one app were not serialized, and a failed swap's rollback message could be wrong and leak staging.
  • F4: writes the running app made during the install were lost.
  • F5: install <id> --version X on an installed app ignored X.
  • F6: backup roots on another filesystem failed, and there was no fallback when the backup location could not be used.

The reviewer's scenarios were rerun against real binaries and all pass.

Compatibility

  • Only the pilotctl binary changes. No daemon or app-store module change is needed, and nothing needs to merge first.
  • Behavior change for scripts: install <id> on an installed app now exits 0 (already_installed: true) instead of exiting 1 with conflict. upgrade --all now exits 1 only after it has tried every app.
  • New files can appear in the install root. Plain .<id>.lock files are never deleted and are harmless. A .app-backups/ dir appears only when the configured and default backup locations both fail. list, outdated, the enterprise installer's managedAppIDs and the supervisor all ignore both; the supervisor logs a skip line for .app-backups.
  • Backups now use disk space beside the install root: the old binary is stripped, and the newest 3 routine backups of each kind are kept.
  • Backups made by dc0e651 builds have no metadata, so they are treated as pinned and never pruned. This only affects dev machines that ran the unreleased first commit.
  • Keep the catalogue freeze. Each node upgrades apps with the pilotctl it already runs, so the stateful-app freeze in the catalogue lint must stay until nodes run a pilotctl with this fix.

Test plan

All pass. Every go command ran with GOWORK=off.

macOS:

  • CI suite go test -short -count=1 -timeout 900s ./pkg/... ./cmd/... ./internal/... with CI's TMPDIR: 22 packages ok, 0 failures.
  • Race suite from the architecture workflow, go test -race -short -parallel 4 -count=1 ./cmd/...: ok. A separate -race run of the appstore tests: ok.
  • go vet ./... and gofmt -s -l: clean.
  • catalogue/lint module: go test and go vet ok.
  • scripts/gen-cli-reference.sh: docs/cli-reference.md unchanged.
  • staticcheck on cmd/pilotctl: no new findings. The 3 remaining ones are older (appstore_sign.go:88, and the unused fatal at main.go:220).
  • gosec on cmd/pilotctl: new findings are annotated with #nosec and a reason. The only one left in touched files is G304 at appstore_update.go:73, on an unchanged line. gosec does not gate CI.
  • GitHub code scanning on the first push reported 2 gosec alerts (G306, G703) on the staged manifest.json write (appstore.go:1443). It is the same write main already has, moved inside the rewritten install path. 4ccbca0 annotates it with #nosec and a reason; comment only. After it: gosec@latest shows no finding on that line, and go vet, the build and go test -short ./cmd/pilotctl pass.

Linux (non-root user pilot):

  • golang:1.25.13-bookworm container: go test -short ./cmd/pilotctl ./internal/enterprisecontrol ok.
  • A cross-compiled test binary run with the appstore test filter: all 30 selected tests pass.

Mutation checks: each of the 13 fixes, reverted on its own, made its test fail.

New tests: appstore_state_regression_test.go (the wipe on main), appstore_state_test.go, appstore_state_review_test.go (F1-F6), appstore_platform_test.go, and catalogue/lint/main_test.go.

Not in this PR

🤖 Generated with Claude Code

teovl and others added 3 commits September 24, 2026 01:24
…tateful catalogue bumps

`pilotctl appstore install --force` and `appstore upgrade` (which the hourly
updater runs as `upgrade --all`) renamed the live app dir to <id>.previous,
swapped in the fresh bundle and RemoveAll'd the old dir. Everything the app
kept in $APP was deleted: the wallet's identity-evm.json (EVM private key)
and data.db, smol's secrets.json, each metered app's identity.json, the
cap-state.jsonl spend-cap ledger and supervisor.log. The next wallet release
in the catalogue would have wiped every installed wallet key within the hour.

Install/upgrade (cmd/pilotctl/appstore_state.go, appstore.go):
- Carry every non-bundle entry of the live dir into staging before the swap:
  hard links (copy fallback), so large data dirs are free and a still-running
  app loses no writes. Not carried: manifest.json, install.json/.sh,
  .sideloaded (set per source), .suspended/.resume, .bundle-sha256,
  next-steps caches, the old binary, sockets/*.sock. Checked in staging first.
- manifest.json is written last, so the supervisor never adopts a
  half-filled staging dir.
- The live dir stays at <id>.previous until the new dir verifies (exact
  manifest, pinned binary sha, carried state); any failure restores it.
- The replaced dir is then moved OUT of the install root (the supervisor
  adopts any manifest-bearing dir there, and a same-version .previous would
  win at daemon start) to app-backups/<id>/<ts>-v<ver>/ beside the root
  ($PILOT_APPSTORE_BACKUP_ROOT overrides). Old binary stripped, small files
  detached from the live inodes, newest 3 kept. Never RemoveAll'd.
- Crash recovery: a lone <id>.previous is restored; a leftover one is backed
  up. Neither is ever deleted.
- `install` of an installed app without --force is now a no-op (exit 0)
  that points at `upgrade` (answered before any download for catalogue ids).
- New `--reset-state` (implies --force) is the explicit destructive case:
  loud stderr warning, state not carried, backup still kept.
- `uninstall` notes remaining backups (they can hold keys).
- JSON report gains already_installed, hint, preserved_state, state_reset,
  backup_dir.

Platform check (item 16, cmd/pilotctl/appstore_platform.go): the existing
thin ELF/Mach-O check now also covers universal Mach-O and PE images, and
the refusal names the app, version and host platform and says nothing was
installed.

Catalogue CI lint (catalogue/lint, .github/workflows/catalogue-lint.yml):
- Fails a PR that updates a stateful app (listed in
  catalogue/stateful-apps.json: wallet, smol, agentphone, bowmark,
  orthogonal; or any app whose old/new bundle manifest grants fs.write or
  key.sign; uninspectable = stateful), for a new version or a same-version
  republish. Override: an approved_bumps entry (id, version, reason,
  approved_by) or the PR label catalogue:stateful-bump-approved. Needed until
  nodes run a pilotctl with this fix, since each node upgrades with its own.
- For added/changed entries, downloads each bundle and checks the sha pin,
  manifest id/app_version, binary pin, and that the binary runs on the
  platform it is published for (a legacy single bundle must not ship a
  native binary). Against the live catalogue it flags cosift 0.1.2 and
  generallegal 0.1.0.
- Documented in catalogue/README.md.

Tests: TestAppStoreForceReinstallKeepsAppState reproduces the wipe on main
(identity-evm.json lost after install --force) and passes here; plus no-op,
--reset-state, carry selection, swap rollback, crash recovery, backup
retention/fallback, signed-catalogue `upgrade --all` end to end, platform
checks (incl. a CLI refusal that leaves the existing install intact), and
catalogue-lint unit tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
F1 carry no longer fails closed on read-only or unreadable state, and
`upgrade --all` goes on past a failed app.
- Carried dirs are created owner-writable while they are filled, then get
  the source mode back (deepest first), so a 0555 dir (a Go module cache)
  carries. Staging, backup and uninstall removal make read-only dirs
  writable first (removeAllForce), so none of them get stuck.
- An entry this user can neither link nor read (a root-owned file under
  fs.protected_hardlinks) is recorded instead of aborting, and moved into the
  new install by rename after the swap (a rename needs no read access).
  Anything that still cannot move stays in the backup, which is then pinned
  and named in a warning and in the report (state_not_carried).
- Any other carry error (disk full, I/O) still aborts with nothing changed.
- `upgrade --all` runs each install with fatal exits trapped
  (fatal_trap.go; fatalCode/fatalHint unwind instead of os.Exit when
  trapped), continues with the next app and exits 1 at the end naming the
  apps that failed (upgrade_failed). A single-app upgrade is unchanged.

F2 retention never removes the only copy of state.
- Every backup gets .pilot-backup.json (kind, versions, pinned). Only
  routine kinds rotate, separately: the newest 3 `upgrade` and the newest 3
  `reinstall` (same-version) backups. `reset-state`, `incomplete` (state
  not carried), `recovered` (crash leftover) and backups without metadata
  are never pruned. Every prune prints a note.

F3 installs of one app are serialized; rollback no longer lies or leaks.
- Per-app flock on <install root>/.<id>.lock (appstore_lock.go, 5 min
  wait with a note) around recovery, carry, swap, reconcile and retire, and
  around uninstall. Released before telemetry and the demo fetch.
- The `install <id>` no-op path is read-only again; only when the live
  manifest is missing and <id>.previous exists does it take the lock (which
  waits out an in-flight swap) before repairing.
- swapInAppDir always discards staging on failure (including when .previous
  already exists or the live dir cannot be moved aside), disabling its
  manifest if it cannot be removed, and reports where the previous install
  actually is. Recovery also discards a leftover <id>.staging.

F4 writes the running app makes during the install are kept.
- After the swap and before retiring, the old dir is walked again: files
  replaced (tmp + rename) or created there since the carry are linked into
  the new install unless its copy changed since (the newer write wins);
  copied files rewritten in place are re-copied; volatile sidecars
  (-journal/-wal/-shm, .lock, .pid, .tmp) the app deleted are dropped (a
  stale rollback journal would roll back a committed transaction).
- Docs no longer claim that hard links alone lose no writes; they state the
  remaining case (writes through a cwd-relative path until the restart).

F5 `install <id> --version X` on an installed app checks X.
- The fast no-op path only answers when X is the installed version. A
  different X without --force fails with `conflict` (exit 1), a version the
  catalogue lacks with `version_unavailable`. A local bundle of another
  version without --force is a `conflict` too.

F6 backup locations and fallbacks.
- A backup root on another filesystem works: EXDEV copies the tree (no old
  binary), then removes the original (manifest disabled first).
- An unusable location falls back to the default beside the install root,
  then <install root>/.app-backups/<id>/ (dot-dir, no manifest: not an app),
  with a warning (also in the report as backup_warning). Only if all fail is
  the dir parked as <id>.previous-<ts> with its manifest disabled; parked
  dirs are stripped, get metadata and are pruned like any other.
- A configured backup root inside the install root is refused.
- `uninstall` lists every remaining backup in every location, parked ones
  included (JSON: backups).

Tests (appstore_state_review_test.go): read-only dirs through 5 reinstalls,
retention and uninstall; an unreadable file and dir moved across; pinned
leftover; `upgrade --all` past a failing app; reset-state and pre-upgrade
backups surviving routine reinstalls; prune rules; lock exclusivity and
timeout; no-op and --force installs waiting out an in-flight swap without
touching it; truthful rollback with staging removed; reconcile unit test
(replace, create, in-place copy rewrite, volatile delete, new-dir wins); an
atomic write during install --force kept; --version conflict /
version_unavailable / no-op / --force; EXDEV copy; the full fallback chain
with pruning, stripping and uninstall listing. Each was checked to fail
with its fix reverted.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Records the app-state fix in [Unreleased] under Fixed: state carried
across install --force / upgrade, backups and retention, the install
no-op, --reset-state, per-app install lock (timeout), upgrade --all
(upgrade_failed), the new install/uninstall JSON fields, the wider
binary platform check and the stateful-app catalogue freeze.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Comment thread cmd/pilotctl/appstore.go Fixed
Comment thread cmd/pilotctl/appstore.go Fixed
GitHub code scanning reported this line as 2 new gosec alerts on
PR #467, because it moved inside the rewritten install path. The finding
is the same one main already has for this write: manifest.json is public
metadata the supervisor reads, written 0644 into
appStoreRoot()/<m.ID>.staging after m.Validate(). Comment only; no
behavior change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@TeoSlayer
TeoSlayer merged commit 8e2ebaa into main Sep 23, 2026
16 checks passed
@TeoSlayer
TeoSlayer deleted the fix/appstore-preserve-app-state branch September 23, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants