Skip to content

Startup reconciliation of stranded app installation states (#124)#178

Open
ClaydeCode wants to merge 6 commits into
mainfrom
fix/clayde/reconcile-stranded-installs
Open

Startup reconciliation of stranded app installation states (#124)#178
ClaydeCode wants to merge 6 commits into
mainfrom
fix/clayde/reconcile-stranded-installs

Conversation

@ClaydeCode

@ClaydeCode ClaydeCode commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Closes #124.

What

Startup reconciliation for app-installation rows stranded by a restart. The install queue is an in-memory asyncio.Queue, so a stop between enqueue and completion leaves installed_apps rows in an in-flight status forever. The uninstall side already got reconcile_interrupted_uninstalls() (#161-era); this adds the install/reinstall side.

New reconcile_interrupted_installs(), run in the lifespan before write_traefik_dyn_config (enqueue-only, so it stays clear of the worker-not-started-yet deadlock window):

  • install with zip on disk → reset to INSTALLATION_QUEUED, re-enqueue "install from zip"
  • store/config/unknown install, zip gone → reset to INSTALLATION_QUEUED, re-enqueue "install from store" (worker re-downloads; self-heals to ERROR if the app is gone)
  • custom install, zip gone → ERROR (source is unrecoverable)
  • reinstall (REINSTALLATION_QUEUED / REINSTALLING) → ERROR. _reinstall_app deletes the app's files before re-downloading, so resuming it unattended could destroy a working install if the store is briefly unreachable at boot; the user retries manually.

The boot-loop half of the issue (a stranded INSTALLING row with missing metadata raising MetadataNotFound out of the lifespan) was already fixed by #158's defensive try/except in write_traefik_dyn_config; agents.md is corrected to match.

Verify

  • tests/test_app_install_crash_recovery.py — 23 tests. Db-tier decision matrix (worker not started, enqueue mocked) asserts DB status + task type for every stranded status; two full-lifespan tests: one proves an unrecoverable row boots and settles to ERROR (store mocked, so a misclassified row would reach STOPPED — the ERROR assertion has teeth), one drives a zip-on-disk install through reconcile → worker → STOPPED.
  • Full local suite: the fix's tests pass. Unrelated failures in test_traefik_dyn_spec / test_tours / test_app_lifecycle reproduce identically on the base commit with this branch's changes reverted and are environmental to the test VM (postgres container collision on :5433, image-pull/timing) — main CI is green. CI on this PR is the real gate.

Recommended reading order

  1. shard_core/app_factory.py — lifespan wiring/order
  2. shard_core/service/app_installation/__init__.py — reconcile_interrupted_installs + helpers
  3. tests/test_app_install_crash_recovery.py
  4. agents.md

Review panel

Ran: adversarial (always), test adversary (real logic added), DevEx/readability (new methods + tests). Skipped — no trigger: security, DB/migration, API-contract, UX.

Adversarial — blocking findings: none. Advisories addressed:

  • Reinstall re-enqueue re-triggers a destructive rmtree-then-download → a transient store outage at boot destroys a working app → fixed (e6a642c): reinstall now settles to ERROR, never re-triggers the destructive path.
  • Reconcile runs after write_traefik_dyn_config; a half-installed INSTALLING row with present metadata gets a stale router (502s that boot) → fixed (e6a642c): reconcile moved before the traefik write, so the not-routable reset takes effect.
  • UNKNOWN reason + no zip marked ERROR instead of resumed → fixed (e6a642c): UNKNOWN resumes from store.
  • Custom install that crashed after extraction but before the STOPPED write is marked ERROR though effectively installed → dismissed: cannot disambiguate "old intact install" from "partial new extraction" without a completion marker; conservative ERROR is safe and the user re-uploads. Narrow window.

Test adversary — blocking findings: none (verified the db-tier tests have teeth by neutering reconcile → 10/10 active-status params failed). Advisories addressed:

  • Integration test's "boots" claim was decoration (Core startup must not crash on one broken app row (unrecoverable boot loop) #158 already prevents the boot break) → fixed (b71824e): renamed to test_unrecoverable_stranded_row_boots_and_settles_to_error, claim scoped to the load-bearing ERROR transition.
  • ERROR could pass for the wrong reason (custom misclassified as store → worker 404 → ERROR) → fixed (b71824e): store is now mocked, so a misclassified row reaches STOPPED and fails the assertion.
  • No reconcile→worker→STOPPED end-to-end test → fixed (b71824e): added test_reconciled_install_completes_on_boot.

DevEx — blocking findings: none. Advisories: added a one-line comment on the load-bearing status reset (b71824e/e6a642c); the flagged _place_zip cross-test cleanup concern was verified moot (path_root is a per-test tmp dir).

This PR description was AI-drafted and human-reviewed. If verifying any part needs real digging on your side, say so and we'll do a call instead.

ClaydeCode and others added 6 commits July 22, 2026 05:39
@ClaydeCode @claude
fix(startup): reconcile stranded app install/reinstall states on boot
923f8a7 
The installation queue lives only in memory, so a restart between enqueue
and completion strands installed_apps rows in INSTALLATION_QUEUED /
INSTALLING / REINSTALLATION_QUEUED / REINSTALLING forever — the uninstall
side already got startup reconciliation (7cf1073) but the install side did
not.

Add reconcile_interrupted_installs(), run right after the uninstall
reconciliation in the lifespan (enqueue-only, so it stays clear of the
worker-not-started-yet deadlock window). For each stranded row:

- install with its zip still on disk -> re-enqueue "install from zip"
- store/config install whose zip is gone -> re-enqueue "install from store"
  (the worker re-downloads)
- custom install whose zip is gone -> mark ERROR; the source is
  unrecoverable, and ERROR stops it looking half-installed and blocking a
  fresh install
- reinstall -> re-enqueue "reinstall"; it always re-downloads from the
  store and the worker self-heals to ERROR if the app is gone

The status is reset to the matching *_QUEUED value before enqueueing so the
worker's status assertion passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode @claude
test(startup): cover install/reinstall crash recovery
6e91dcc 
Decision-matrix tests (db tier, worker not started) assert the resulting
DB status and the enqueued task type for every stranded status, plus that
settled apps are left untouched. A full-lifespan test proves a row stranded
in INSTALLATION_QUEUED / INSTALLING with missing files no longer boot-loops
core and ends in ERROR.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode @claude
docs(agents): document install/reinstall startup reconciliation
7d0e0b2 
Record reconcile_interrupted_installs() alongside the uninstall equivalent,
and correct the now-stale claim that a missing-metadata row boot-loops core
(write_traefik_dyn_config skips it defensively since #158).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode @claude
fix(startup): make reinstall recovery non-destructive, run reconcile …
e6a642c 
…before traefik

Review-panel follow-ups:

- Reinstall is now settled to ERROR instead of re-enqueued. _reinstall_app
  deletes the app's files before re-downloading, so resuming it unattended
  could destroy a working install if the app store is briefly unreachable at
  boot — exactly the case the API's app_exists_in_store precheck guards. ERROR
  is a non-destructive terminal state the user retries manually.
- reconcile_interrupted_{uninstalls,installs} now run before
  write_traefik_dyn_config, so a row reset to a non-routable status (e.g.
  INSTALLING -> INSTALLATION_QUEUED) is excluded from the boot's Traefik config
  rather than briefly routed at a half-installed container.
- An UNKNOWN installation_reason with no zip is resumed from the store (the
  worker self-heals to ERROR if it is not there) rather than marked ERROR.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode @claude
test(startup): strengthen install crash-recovery coverage
b71824e 
- Reinstall statuses now assert ERROR (non-destructive settle) rather than
  re-queue.
- Add a reconcile->worker->STOPPED lifespan test that drives a zip-on-disk
  install to completion, pinning that reconcile's task_type and status reset
  agree with the worker's status assertion end to end.
- Mock the app store in the ERROR lifespan test so a misclassified custom
  install would reach STOPPED, giving the ERROR assertion teeth; extend it to
  the reinstall statuses.
- Cover the zip-precedence-over-reason and UNKNOWN-reason branches.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode @claude
docs(agents): reflect reinstall-to-ERROR and reconcile ordering
a4d4c97 
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@ClaydeCode
ClaydeCode requested a review from max-tet July 22, 2026 05:57
@ClaydeCode ClaydeCode mentioned this pull request Jul 22, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@max-tet max-tet Awaiting requested review from max-tet

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Startup reconciliation of stranded app installation states

1 participant

@ClaydeCode