Skip to content

Add storage-aware app install admission - #785

Closed
dzanahmed wants to merge 4 commits into
mobius-os:mainfrom
dzanahmed:enhancement/app-install-storage-quota
Closed

Add storage-aware app install admission#785
dzanahmed wants to merge 4 commits into
mobius-os:mainfrom
dzanahmed:enhancement/app-install-storage-quota

Conversation

@dzanahmed

@dzanahmed dzanahmed commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Estimate an app's install footprint from the complete candidate package during opt-in previews.
  • Report writable installation headroom while preserving Möbius's existing disk safety margin.
  • Reject installs server-side when the current storage budget cannot accommodate them.
  • Keep ordinary capability previews manifest-only unless a client requests size planning.
  • Publish pre-computed package-size metadata (exact declared payload, conservative installed-footprint estimate, content digest) in a versioned package-metadata.json feed so clients can display instant size estimates without downloading any app files.

Safety

  • The final quota check runs on freshly fetched bytes immediately before installation mutates state.
  • The reserved disk margin remains unavailable to app installs so database and owner data retain working headroom.
  • Existing install review bindings and atomic rollback behavior remain unchanged.
  • Package metadata is pre-computed at release time and versioned; it does not alter the discovery catalog.

Testing

  • pytest -q backend/tests/test_resource_pressure.py backend/tests/test_app_install_budget.py (13 passed)
  • python3 -m py_compile backend/app/resource_pressure.py backend/app/schemas.py backend/app/install.py backend/app/routes/apps.py

Co-authored-by: Möbius Agent <mobius-agent@users.noreply.github.com>
@hamzamerzic

Copy link
Copy Markdown
Collaborator

Möbius agent review

Result: No blocking issue found. Opt-in previews calculate the complete candidate footprint, the final serialized install path rechecks fresh bytes against the existing reserve, and ordinary capability previews remain lightweight. The focused budget and pressure suites passed (13 tests). Cross-repository rollout note: App Store PR #30 relies on this preview response for batch-install approval, so merge and deploy this platform change first.

I reviewed the full diff, current description and discussion, and the available checks for this revision.

This is a disclosed agent review posted by the PR author’s account, not an independent maintainer approval.

Precompute and validate versioned package footprints, measure installed apps locally for uninstall previews, and parallelize authoritative package review without weakening quota enforcement.\n\nCo-authored-by: Möbius Agent <mobius-agent@users.noreply.github.com>
@dzanahmed

Copy link
Copy Markdown
Contributor Author

Coordinated change

This PR has a companion in the App Store: mobius-os/app-store#30 (Add batch install and uninstall management).

The two ship together:

  • This platform change enforces the storage quota server-side as the final guard before any install commits.
  • The App Store change provides the batch selection UI, per-app progress, and the versioned package-metadata.json feed that makes size estimates instant.

Either can land first — the platform quota guard is additive and the Store degrades gracefully when the feed is absent — but both should be reviewed and merged in the same window to keep the user-facing experience consistent.

@hamzamerzic hamzamerzic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This combines several independently valuable changes—bounded parallel package fetching, install admission, release metadata validation, per-app footprint measurement, and new API projections—into one coupled contract. Please split them so each owning seam can be reviewed and shipped independently. The parallel-fetch refactor is a strong standalone change.

The hard install rejection cannot rely on max(1 MiB, fetched_payload * 3): compilation, normalized assets, Git objects, filesystem allocation, and package shape can exceed that heuristic, so a 507 based on it can claim safety it does not prove. Keep estimates advisory, or enforce admission from the exact staged/materialized result on the target filesystem while preserving the existing reserve. The server must not use an unproven multiplier as its safety boundary.

package_footprint validation and the separate catalog package-metadata.json feed in the companion Store PR also create two size-metadata authorities. Choose one reviewed, version-bound source rather than maintaining both. After splitting, retain the bounded-concurrency cancellation tests and add a hard-admission test proving the measured/staged bytes—not a heuristic—own the final decision.

@miljanm miljanm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: QA second look

I found the following concrete risks. I’ve kept this focused on issues with a supported failure mode rather than style preferences.

MEDIUM · source_files fully buffered before the aggregate byte cap is enforced (lost early abort)

backend/app/install.py:2100 · correctness
Reviewer rule: correctness.concrete_regression

A hostile catalog/remote manifest (also reachable through the opt-in include_install_size preview via get_owner_or_app_with_manage_apps) can declare a very large number of source_files, each up to the per-file _ENTRY_MAX_BYTES limit. Because there is neither a count cap nor an incremental abort, all declared files are fetched and retained in memory before the aggregate _SOURCE_FILES_TOTAL_MAX check can reject them, so peak memory/bandwidth becomes N x per-file limit instead of the previously-bounded total. The old sequential path aborted after roughly _SOURCE_FILES_TOTAL_MAX bytes, so this is a concrete regression enabling OOM/bandwidth exhaustion.

Evidence: The rewrite schedules every declared source file at once: source_tasks = {rel: asyncio.create_task(fetch_input(rel, _ENTRY_MAX_BYTES)) for rel in manifest.get('source_files') or []}, awaits asyncio.gather(*fetch_tasks), then builds source_files = {rel: task.result() ...} and only afterward runs if sum(map(len, source_files.values())) &gt; _SOURCE_FILES_TOTAL_MAX: raise HTTPException(400, ...). This replaces the old loop for rel ...: data = await _http_get(...); source_files_total += len(data); if source_files_total &gt; _SOURCE_FILES_TOTAL_MAX: raise. The same hunk adds pre-fetch count caps for the other lanes (if len(static_entries) &gt; _STATIC_ASSETS_COUNT_MAX and `if len(see…

Suggested direction: Enforce a source_files count cap before scheduling fetches (mirroring static_assets/seeds), or accumulate the running byte total as task results resolve and cancel/abort once _SOURCE_FILES_TOTAL_MAX is crossed, so buffering stays bounded rather than proportional to the number of declared files.

MEDIUM · New 507 install-quota admission gate is left unprotected by tests

backend/app/install.py:2735 · tests
Reviewer rule: tests.changed_behavior_unprotected

The central new safety behavior — refusing an install when the storage budget cannot accommodate it — has no regression protection. A later change to the comparison direction, the check's ordering relative to mutation, or the budget wiring could silently disable admission and let installs consume the reserved disk margin without any failing test.

Evidence: install_from_manifest now computes estimated_install_bytes = candidate_install_bytes(candidate) and budget = app_install_storage_budget(get_settings().data_dir), then if estimated_install_bytes &gt; budget['available_bytes']: raise HTTPException(507, {'code': 'app_install_quota_exceeded', ...}) before any DB/filesystem mutation. The added tests only cover the pure candidate_install_bytes helper (test_app_install_budget.py), the app_install_storage_budget helper (test_resource_pressure.py), the /install-budget endpoint, the /footprint endpoint, and _fetch_install_candidate concurrency (test_apps_install.py). None drive install_from_manifest into the over-budget branch.

Suggested direction: Add an install-path test that stubs app_install_storage_budget to report insufficient available_bytes, asserts install_from_manifest raises HTTPException(507) with the app_install_quota_exceeded payload, and verifies no DB/filesystem mutation occurred.

Reviewed revision d3776f65bdb8.

Resolved one conflict in backend/tests/test_apps.py: kept both the
footprint projection test added by this branch and the new source
manifest contract test added upstream.

Co-Authored-By: Möbius Agent <mobius-agent@users.noreply.github.com>

@miljanm miljanm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: QA second look

I found the following concrete risks. I’ve kept this focused on issues with a supported failure mode rather than style preferences.

MEDIUM · source_files fully buffered before the aggregate byte cap is enforced (lost early abort)

backend/app/install.py:2100 · correctness
Reviewer rule: correctness.concrete_regression

A hostile catalog/remote manifest (also reachable through the opt-in include_install_size preview via get_owner_or_app_with_manage_apps) can declare a very large number of source_files, each up to the per-file _ENTRY_MAX_BYTES limit. Because there is neither a count cap nor an incremental abort, all declared files are fetched and retained in memory before the aggregate _SOURCE_FILES_TOTAL_MAX check can reject them, so peak memory/bandwidth becomes N x per-file limit instead of the previously-bounded total. The old sequential path aborted after roughly _SOURCE_FILES_TOTAL_MAX bytes, so this is a concrete regression enabling OOM/bandwidth exhaustion.

Evidence: The rewrite schedules every declared source file at once: source_tasks = {rel: asyncio.create_task(fetch_input(rel, _ENTRY_MAX_BYTES)) for rel in manifest.get('source_files') or []}, awaits asyncio.gather(*fetch_tasks), then builds source_files = {rel: task.result() ...} and only afterward runs if sum(map(len, source_files.values())) &gt; _SOURCE_FILES_TOTAL_MAX: raise HTTPException(400, ...). This replaces the old loop for rel ...: data = await _http_get(...); source_files_total += len(data); if source_files_total &gt; _SOURCE_FILES_TOTAL_MAX: raise. The same hunk adds pre-fetch count caps for the other lanes (if len(static_entries) &gt; _STATIC_ASSETS_COUNT_MAX and `if len(see…

Suggested direction: Enforce a source_files count cap before scheduling fetches (mirroring static_assets/seeds), or accumulate the running byte total as task results resolve and cancel/abort once _SOURCE_FILES_TOTAL_MAX is crossed, so buffering stays bounded rather than proportional to the number of declared files.

MEDIUM · New 507 install-quota admission gate is left unprotected by tests

backend/app/install.py:2735 · tests
Reviewer rule: tests.changed_behavior_unprotected

The central new safety behavior — refusing an install when the storage budget cannot accommodate it — has no regression protection. A later change to the comparison direction, the check's ordering relative to mutation, or the budget wiring could silently disable admission and let installs consume the reserved disk margin without any failing test.

Evidence: install_from_manifest now computes estimated_install_bytes = candidate_install_bytes(candidate) and budget = app_install_storage_budget(get_settings().data_dir), then if estimated_install_bytes &gt; budget['available_bytes']: raise HTTPException(507, {'code': 'app_install_quota_exceeded', ...}) before any DB/filesystem mutation. The added tests only cover the pure candidate_install_bytes helper (test_app_install_budget.py), the app_install_storage_budget helper (test_resource_pressure.py), the /install-budget endpoint, the /footprint endpoint, and _fetch_install_candidate concurrency (test_apps_install.py). None drive install_from_manifest into the over-budget branch.

Suggested direction: Add an install-path test that stubs app_install_storage_budget to report insufficient available_bytes, asserts install_from_manifest raises HTTPException(507) with the app_install_quota_exceeded payload, and verifies no DB/filesystem mutation occurred.

Reviewed revision 714ca2d16ca3.

@miljanm miljanm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: QA second look

I found the following concrete risks. I’ve kept this focused on issues with a supported failure mode rather than style preferences.

MEDIUM · source_files lane loses its incremental byte early-abort and has no count cap, so buffering scales with declared file count

backend/app/install.py:2100 · correctness
Reviewer rule: correctness.concrete_regression

A hostile catalog/remote manifest — reachable pre-install through the opt-in include_install_size preview under manage_apps — can declare many source_files, each up to _ENTRY_MAX_BYTES. With neither a count cap nor an incremental abort, every declared file is fetched and retained in memory before the aggregate check can reject the manifest, so peak memory/bandwidth scales with the declared file count (up to N × _ENTRY_MAX_BYTES) instead of the previously bounded _SOURCE_FILES_TOTAL_MAX, enabling owner-instance memory/bandwidth exhaustion.

Evidence: The rewrite schedules every declared source file eagerly — source_tasks = {rel: asyncio.create_task(fetch_input(rel, _ENTRY_MAX_BYTES)) for rel in manifest.get('source_files') or []} — awaits asyncio.gather(*fetch_tasks), materializes source_files = {rel: task.result() ...}, and only then runs if sum(map(len, source_files.values())) &gt; _SOURCE_FILES_TOTAL_MAX: raise HTTPException(400, ...). The prior loop accumulated source_files_total += len(data) and aborted mid-stream once _SOURCE_FILES_TOTAL_MAX was crossed. The same hunk keeps pre-fetch count caps for the other lanes (if len(static_entries) &gt; _STATIC_ASSETS_COUNT_MAX, if len(seed_entries) &gt; _SEEDS_COUNT_MAX) but…

Suggested direction: Enforce a source_files count cap before scheduling fetches (mirroring static_assets/seeds), or accumulate the running byte total as results resolve and cancel remaining tasks once _SOURCE_FILES_TOTAL_MAX is crossed, so buffering stays bounded rather than proportional to the declared file count.

MEDIUM · New 507 install-quota admission gate is left unprotected by tests

backend/app/install.py:2735 · tests
Reviewer rule: tests.changed_behavior_unprotected

The central new safety behavior — refusing an install when the storage budget cannot accommodate it — has no regression protection. A later change to the comparison direction, the check's ordering relative to mutation, or the budget wiring could silently disable admission and let installs consume the reserved disk margin without any failing test.

Evidence: install_from_manifest now computes estimated_install_bytes = candidate_install_bytes(candidate) and budget = app_install_storage_budget(get_settings().data_dir), then if estimated_install_bytes &gt; budget['available_bytes']: raise HTTPException(507, {'code': 'app_install_quota_exceeded', ...}) before any DB/filesystem mutation. The added tests exercise only the pure helper candidate_install_bytes (test_app_install_budget.py), app_install_storage_budget (test_resource_pressure.py), the /install-budget and /{app_id}/footprint endpoints, and _fetch_install_candidate concurrency (test_apps_install.py). None drive install_from_manifest into the over-budget branch.

Suggested direction: Add an install-path test that stubs app_install_storage_budget to report insufficient available_bytes, asserts install_from_manifest raises HTTPException(507) with the app_install_quota_exceeded payload, and verifies no DB/filesystem mutation occurred.

Reviewed revision 714ca2d16ca3.

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