Repository navigation
sbx-mgr store: typed reads and named scans - #715
Merged
Merged
Conversation
ac-pplx
force-pushed
the
ac/sbxmgr-store-typed-reads
branch
from
October 20, 2026 18:03
8d37855 to
cb0746d
Compare
ac-pplx
force-pushed
the
ac/sbxmgr-store-typed-reads
branch
from
October 20, 2026 18:33
cb0746d to
712b4b2
Compare
ac-pplx
marked this pull request as ready for review
October 20, 2026 18:38
ac-pplx
force-pushed
the
ac/sbxmgr-store-typed-reads
branch
from
October 20, 2026 19:42
712b4b2 to
2aa8b30
Compare
ac-pplx
force-pushed
the
ac/sbxmgr-store-typed-reads
branch
2 times, most recently
from
October 20, 2026 22:12
fe95b1c to
41c1c21
Compare
Add the consolidated read API beside the store's existing readers. No
caller changes and no behavior change: every new read or scan is one call
to the reader it replaces, preserving that reader's statement and
transaction sequence, decoder, missing-row rule and strictness. A follow-up converts sbx-mgr's callers
and narrows the old readers.
`db::read`, the typed reads:
- `sandbox` (strict whole row, today's `get_sandbox`) and
`sandbox_within_budget` (`inventory_read::get`, same byte charging),
with the two raw projections a whole row cannot stand in for:
`state_changed_at_witness` and `route_identity_adoption`. The pure
row predicates become `SandboxRow` methods: `has_runtime_fields`,
`is_retryable_interrupted_restore`,
`cleanup_intent_for_startup_recovery` and `erase_safety`.
- `sandbox_view(id)` and `sandbox_views(ids)`: narrow accessors that
work without a row, and `typed_sandbox_view(SandboxId)` for the
auto-lifecycle delete receipts the outbox keys by typed id. Core
accessors: `current_state_label`, `runtime_execution_generation`,
`node_execution_fence` (with the previous execution),
`cleanup_target`, `cleanup_runtime_label`, `cleanup_intent_current`,
`activation_route_provenance`, `exists`,
`suspended_with_pinned_artifact` and `state_labels`. Facets:
- `cleanup_episode`: `present`, `strict`, `decoded`
(`Decoded<Absent | Present | Undecodable>`, row-decode errors only)
and `current`;
- `restore_scope`: `scope` and `reclaim_attempt_is_current` (strict
on the whole row, then the scope);
- `delete_receipts`: `execution_covers` with the caller's clock; on
the typed view only, `pending` and
`pending_matches_observed_execution`, so a text view cannot reach
them at all;
- `teardown`: `latest`, `status`, `running_for`, `release_evidence`,
`retry_due` and `retry_admissible`;
- `quarantine`: `is_orphan_recovery_state` and `claim_is_current`;
- `route_claim`, `pinned_artifact`, `latest_upload`, and `artifacts`
(`named`, budgeted, and `others_awaiting_backup`).
- `state_page(ids, budgets)`: one State RPC batch in request order with
per-row, per-field results, charging the metadata budget first and
then the raw budget, exactly as the State RPC does today.
- `local_artifact(Id | Named)` and `local_artifact_unowned`,
`cleanup_job`, and `counts(ByLifecycle | Resident |
MalformedSuspendedUpdatedAt | DeleteOutbox | DeleteBlocked)`.
`db::scan`, the named scans, each keeping its own pinning, filter, cap,
budget and malformed-row policy: `sandbox_pages` (Inventory,
CleanupTargets, ResidentRecovery, and `CleanupObligationScan`, which has
no candidate cap), `all_sandboxes` (StrictRows, SkipUndecodableRows,
RpcListRows, GuestHealthRows), `list_page`, `resolve_selector`
(RpcSelector, InventorySelector), `by_peer` (NetGateway, SpacedHost),
`unbound_executions`, `pool_donor`, `quarantine_recovery`,
`eviction_candidates` (Proofable, ProoflessLegacy),
`due_auto_lifecycle`, `node_dirty_batch`, `rowless_restore_scopes`
(Cursor, All), `local_artifacts` and `pending_backups`. The variant
traits are sealed.
`execution_delete_receipt_covers` now reads the clock and calls the new
accessor, so the retention-guarded statement has one copy. The
foreign-key comments in `db.rs`, a store test and the crate's AGENTS.md
are corrected: the bundled SQLite enables foreign keys by default and the
store never turns them off; only some child tables declare
`ON DELETE CASCADE`.
Tests:
- Every view accessor is characterized against its old reader over
seven sandboxes (requested cleanup over a live runtime, suspended with
an owned pin, two interrupted remote restores, held recovery claim,
orphan quarantine, and a paused row with an older pending receipt and
an expired execution receipt): an accepting fixture where each
accessor finds or agrees with something, with refusing rows for the
two receipt checks; corrupt JSON in every row but one restore, plus
one undecodable child record each (cleanup episode, teardown payload,
both restore scopes, quarantine reason codes); every row removed by a
raw DELETE with only the foreign-key cascades (not the production
delete path), so episodes, claims and quarantines go and the other
child tables keep their rows; and a previous node execution without a
current one. Each answer is stated in the table, and the accessor and
the old reader must both give it.
- `state_page`'s smallest raw and metadata byte allowances equal the old
State sequence's, each budget is charged one item per existing row,
and with both budgets too small the metadata preflight fails first.
- `CleanupObligationScan` pages through one pass of 130 obligations in
two pages and then starts over.
- Mutation checks, each failing for the named reason and then reverted:
swapping the id arguments of `suspended_with_pinned_artifact` or of
`owned_evidence`; a constant `Ok(None)` for `pending`; a constant
`false` for either quarantine accessor; a constant `Ok(true)` for
`pending_matches_observed_execution` or `execution_covers`; charging
rows before metadata in `state_page`; and, from
the first round, a strict `cleanup_target`, a fatal `decoded`, `>` for
`>=` in `execution_covers` and rows charged to the metadata budget.
ac-pplx
force-pushed
the
ac/sbxmgr-store-typed-reads
branch
from
October 21, 2026 04:38
41c1c21 to
4dc2b0e
Compare
nickdujay-pplx
approved these changes
Oct 21, 2026
nickdujay-pplx
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the typed readers and scans, including missing/corrupt rows, execution fences, cleanup paging, and State RPC budget/error ordering. The wrappers preserve the underlying reads and leave persisted formats unchanged.
Validated with #721 on top: 384 store tests and 1,407 sbx-mgr tests passed, along with a production-library compile check without test-only store visibility. No correctness, deployment, or rollback blocker found.
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
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.
Summary
Step 2 of consolidating
pplx-space-sbx-mgr-store's read API: add the typed reads (db::read) and named scans (db::scan) beside the existing readers. Additive only: no caller changes, no behavior change. Each accessor is one call to its old reader, preserving that reader's statement and transaction sequence, missing-row rule and strictness. The one stated exception isdelete_receipts().execution_covers, which owns the statement: the oldexecution_delete_receipt_coversreads the clock and calls it. The stacked follow-up converts sbx-mgr's callers and makes the replaced readers non-public.read::sandbox,read::sandbox_within_budget,read::state_changed_at_witness,read::route_identity_adoption;SandboxRow::{has_runtime_fields, is_retryable_interrupted_restore, cleanup_intent_for_startup_recovery, erase_safety}get_sandbox,inventory_read::get,sandbox_state_changed_at_witness,auto_lifecycle_delete_receipts::route_identity_adoption_metadata,sandbox_has_runtime_fields,sandbox_is_retryable_interrupted_restore,cleanup_intent_for_startup_recovery,sandbox_erase_safetyread::sandbox_view(&str),read::sandbox_views(&[String]):current_state_label,runtime_execution_generation,node_execution_fence,cleanup_target,cleanup_runtime_label,cleanup_intent_current,activation_route_provenance,exists,suspended_with_pinned_artifact,state_labelssandbox_exists,sandbox_suspended_with_pinned_artifact,guest_health_scan::state_labels_for_idscleanup_episode()present,strict,decoded(Decoded<T>; only row-decode errors becomeUndecodable),currenthas_cleanup_episode,cleanup_episode,cleanup_episode_currentrestore_scope()scope,reclaim_attempt_is_currentrestore_transfer_scope,canonical_restore_reclaim_attempt_is_currentdelete_receipts()execution_covers(gen, now); onread::typed_sandbox_view(SandboxId)only:pending,pending_matches_observed_executionauto_lifecycle_delete_receipts::{pending_receipt_for_sandbox, pending_receipt_matches_observed_execution},execution_delete_receipt_coversteardown()latest,status,running_for,release_evidence,retry_due,retry_admissiblelatest_teardown_suspend_operation,teardown_suspend_status,teardown_suspend_operation_running_for,teardown_release_evidence,teardown_suspend_retry_due,teardown_suspend_retry_admissiblequarantine()is_orphan_recovery_state,claim_is_currentis_orphan_quarantine_recovery_state,quarantine_recovery_claim_is_currentroute_claim().matches,pinned_artifact().{pin_and_updated_at, owned_evidence},latest_upload,artifacts().{named, others_awaiting_backup}route_activation_claims::claim_matches,eviction_scan::{artifact_pin_and_updated_at, owned_artifact_evidence, latest_upload_job},local_artifact_leases::list_named_publications,other_local_artifacts_awaiting_backupread::state_page(ids, &mut StateBudgets)→StatePageRow::{Missing, Decoded, StateOnly}in request order, per-fieldResults;StatePageErrorinventory_read::check_state_metadata,activation_route_provenance_for_sandboxes,sandbox_state_publication_metadata,sandbox_state_changed_at,quarantine_recovery_observations,raw_sandbox_lifecycle(andinventory_read::getwithin the batch); it also calls #620'spinned_local_artifact_created_at, which stays publicread::local_artifact(LocalArtifactKey::{Id, Named}),read::local_artifact_unownedget_local_artifact,get_named_local_artifact,local_artifact_leases::gc_staging_is_unownedread::cleanup_jobcleanup_jobs::get_jobread::counts(ByLifecycle | Resident | MalformedSuspendedUpdatedAt | DeleteOutbox | DeleteBlocked)sandbox_lifecycle_counts,eviction_scan::{count_resident_rows, count_malformed_suspended_updated_at},auto_lifecycle_delete_receipts::{outbox_stats, blocked_stats}scan::sandbox_pages(&mut cursor)forSandboxInventoryScan,CleanupTargetScan,PeriodicLifecycleRecoveryScan,CleanupObligationScannext_batch,periodic_lifecycle_recovery_batch,cleanup_recovery_pagescan::all_sandboxes(StrictRows | SkipUndecodableRows | RpcListRows { rpc } | GuestHealthRows)list_sandboxes,list_sandboxes_for_recovery,inventory_read::list,guest_health_scan::list_guest_health_rowslist_page,resolve_selector(RpcSelector | InventorySelector),by_peer(Peer::{NetGateway, SpacedHost}),unbound_executions,pool_donor,quarantine_recovery,eviction_candidates(Proofable | ProoflessLegacy),due_auto_lifecycle,node_dirty_batch,rowless_restore_scopes(Cursor | All),local_artifacts,pending_backupsinventory_read::{list_page, lookup},find_by_prefix,list_by_{netgtw,spaced_host}_pid_uid,unbound_node_executions,find_pool_sandbox,PeriodicQuarantineRecoveryScan::next_batch,eviction_scan::{idle_suspended_candidates, legacy_reupload_candidates},due_auto_lifecycle_batch,list_node_dirty_refs,RestoreTransferScopeScan::next_batch,rowless_restore_transfer_scopes,ArtifactScan::next_page,local_artifact_leases::list_pending_backupsVariants with different outputs (
counts,all_sandboxes,resolve_selector,eviction_candidates,sandbox_pages) are unit or cursor types behind a sealed trait, so each keeps its exact return and error type and callers cannot add their own; variants that share an output type (by_peer,rowless_restore_scopes,local_artifact) are enums.Contract notes
restore_scope().reclaim_attempt_is_currentdecodes the whole row strictly, then reads the scope;teardown().status/release_evidencedecode the row's state and artifact pin. These also read thesandboxesrow:cleanup_episode().current,teardown().retry_admissible(row plus episodes),quarantine().is_orphan_recovery_state,quarantine().claim_is_current(state plus episodes, through the claim fence's sandbox guards) and the typed view'spending_matches_observed_execution.SandboxId, sopendingandpending_matches_observed_executionexist only ontyped_sandbox_view(SandboxId): a text view cannot reach them at compile time.quarantine().claim_is_currentis a pure delegation; its claim names its own sandbox, and the caller opens the view on it.state_pagecharges the metadata preflight first, then each row's raw bytes, and returns rows in request order; the State RPC publishes state-only rows first, so a caller reproducing it partitions.CleanupObligationScanhas no 8-of-128 cap; the design text was wrong there. It returns every qualifying id among the next 128 rowids, unpinned, in one deferred transaction (today'scleanup_recovery_pagewith the 128 its only caller passes). The cursor is private andCopy.resolve_selector(InventorySelector)charges the exact-id read and every examined id to one budget ofMAX_ITEMS(4096) items andMAX_RAW_BYTES(16 MiB); the unique match is then read with a fresh budget.SQLITE_DEFAULT_FOREIGN_KEYS=1and the store never turns it off (open_rusqliteadds no pragma; itsforeign_keysoption defaults tofalse, meaning "don't addPRAGMA foreign_keys=ON", not "off"). The comment ondelete_sandbox_child_rowsand the crate's AGENTS.md said otherwise and are corrected.Design decisions taken without the operator (unattended)
SandboxSnapshotisSandboxRow: a second name for the same type, or a rename across ~300 sites, is not a no-behavior-change step.Facetsselection, so each call is one call to an old reader and no converted caller runs a read it did not run before.Tests
every_view_accessor_answers_like_its_old_reader: 38 accessor cases × 4 fixtures over seven sandboxes: requested cleanup over a live runtime with a running teardown job, retained execution receipt, written route claim and pending delete receipt; suspended with an owned pinned artifact, an upload job and an un-backed-up named publication; two interrupted remote restores with their scopes; a held legacy-recovery claim; an orphan quarantine; and a paused row with a pending receipt for an older generation and an expired execution receipt.DELETEwith only the foreign-key cascades, not the production delete path (which first runsdelete_sandbox_child_rows). Episodes, claims and quarantine state cascade; jobs, artifacts, scopes and execution receipts remain. Pending receipts guard their rows, so their outbox rows are deleted first.Each answer is stated independently in the table, and the accessor and the old reader must both give it.
state_page_charges_both_budgets_exactly_as_the_state_rpc_did: smallest raw and metadata byte allowances equal the old sequence's; one item per existing row; with both budgets too small the error isMetadata(preflight first).cleanup_obligations_page_through_one_pass_and_start_over: 130 obligations → pages of 128 and 2, then a new pass.Mutation checks (applied, run, reverted):
suspended_with_pinned_artifactsuspended_with_pinned_artifact on Acceptingpinned_artifact().owned_evidencepinned_artifact.owned_evidence on Acceptingpending()returns a constantOk(None)delete_receipts.pending on Acceptingpending_matches_observed_executionreturns constantOk(true)delete_receipts.pending_matches_observed_execution (older receipt) on Acceptingexecution_coversreturns constantOk(true)delete_receipts.execution_covers (expired receipt) on Acceptingis_orphan_recovery_statereturns constantfalsequarantine.is_orphan_recovery_state on Acceptingclaim_is_currentreturns constantfalsequarantine.claim_is_current on Acceptingstate_pagecharges rows before the metadata preflightMetadataassertioncleanup_targetdecodes the whole rowcleanup_target on CorruptJsondecoded()treats decode errors as fatalcleanup_episode.decoded on CorruptJsonexecution_coversuses>for>=delete_receipts.execution_covers on Acceptingstate_pagecharges rows to the metadata budgetValidation
Rebased onto main after #717 (and #543/#700); no conflicts. #543 adds no store reader and changes none that an accessor delegates to (its one new store function,
publish_restored_stopped_artifact, is a writer), so every accessor is unchanged.Rebased onto main after #620;
state_pagegains one delegated read, #620'spinned_local_artifact_created_at, run between the state-change and recovery reads as #620's State RPC runs it, returned asDecodedStateRow::pinned_artifact_created_at, withStatePageError::PinnedArtifactCreatedAtcarrying #620's message, so that #721 can keep that field onstate_page. The reader stays public with no new accessor. #620 changes no reader an accessor delegates to (its other store changes are writers).Rebased onto main after #714 and #718 (and #734/#701). One conflict, in
db.rs: #714 added to thedelete_sandbox_child_rowsdoc comment that it deliberately skipsnetwork_policy_reconcile_queue, which nothing writes or reads any more and a later release drops. The merged comment keeps this PR's foreign-key correction and ends with #714's sentence verbatim. Neither PR removes or changes a reader an accessor delegates to: #714's store removals (update_runtime_fields, now a test fixture, and the reconcile-queue writes) and #718's (delete_named_local_artifact) are writers, so every accessor is unchanged. #714's newhas_live_network_policy_gatewayreader stays public with no accessor, likepinned_local_artifact_created_at.bash ci/scripts/run-rust-checks.sh fmtbash ci/scripts/run-rust-checks.sh clippy(workspace, all targets/features, sbx-mgr production checks)cargo arch(against main)SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked --all-features -p pplx-space-sbx-mgr -p pplx-space-sbx-mgr-store -p pplx-space-db-migrate -p pplx-space-db-migrator -p pplx-space-db-test-fixtures(once)transfer_directory::tests::distinct_service_uids_can_exchange_downloads: ignored, asserts a root runner; untouched code)RUSTDOCFLAGS=-Dwarnings cargo doc --locked --no-deps -p pplx-space-sbx-mgr-storepinned_local_artifact_created_atlinks the privateinsert_local_artifact; none from this PR🤖 Generated with Claude Code