[ac-pplx]

Summary

The admin sandbox list (GET /admin/api/v1/sandboxes) returned at most the newest 5,000 rows, filtered only by org_id. The admin console could therefore reach an older sandbox only by exact lookup, and filtered everything else in the browser. Admin sandbox routes also took different identifier forms. Merging this:

1. Pages the list by cursor.

   • The order is unchanged: created_at descending, then the ID.
   • Passing cursor returns {sandboxes, next_cursor}. An empty cursor asks for the first page, and next_cursor is null on the last page.
   • The limit and the estimated byte budget (8 MiB, as for the ECW list's pages) apply to the rows from the database, and the first such row is kept even when it is larger than the budget. The rows nodes report without a database row are all appended to the first unfiltered page, outside those bounds. The estimate does not count each row's network policy: counting it would change the page walk the ECW list shares.
   • The cursor is an opaque, unsigned keyset position (the last row's created_at and ID). It only marks a position in a list the caller can already read in full.
   • Each page reads the current matching rows strictly below the supplied (created_at, id) position. Paging is not a snapshot: concurrent changes can affect membership. created_at is the inserting transaction's now(), so a transaction that began before a page boundary and commits later lands on a later page. Keep the filters unchanged between pages.
   • Without cursor the route answers the bare array as before: a plain list statement with no byte budget and no size estimate, because it has no cursor to continue from.
   • The node-reported rows with no sandbox row, such as warm-pool slots, are appended only to the unscoped list (since api, ui: show and filter sandboxes by organization #1248). They come on the first page of the unfiltered list only.

2. Adds server-side filters that combine with org_id:

   • lifecycle: comma-separated values of the stored lifecycle, including deleted and cleanup_required. No response field shows it. The response's lifecycle and status report the node's observed status when there is one, so a row can show another value. Filtering on what the response shows would mean filtering on free-form node status text joined from observed state, so the filter keeps to the validated stored vocabulary.
   • template: the row's own template. A row without one does not match, although the response's template and the count fall back to the template its node reports.
   • No node filter. Matching the response's active_node_id (the direct route's current node, else the row's) means reading the Direct-only route table, which no owning read covers yet, so node_id and assigned are left out. The console can list a node's sandboxes with the existing GET /nodes/{id}/sandboxes. The two filters can follow once the Direct route has an owning read.
   • user_id.
   • created_after and created_before: RFC 3339, both exclusive. Postgres keeps microseconds, so a created_before inside a microsecond rounds up, and a row created earlier in that microsecond still matches.
   • prefix: a case-sensitive name prefix, or an ID prefix with optional sbx_ and dashes where a UUID has them. A NUL character answers 400.

   Every filter, org_id included, leaves out the node-reported rows: they belong to no org, and none of the filters can be applied to them.

   Malformed values, typed ones (limit, org_id, user_id) included, answer 400 with the admin JSON {"error": …} rather than axum's plain-text rejection. With cursor, an unknown query parameter answers 400, so a misspelt filter cannot return unfiltered pages. Without cursor, unknown parameters are still ignored for existing callers.

3. Gives the admin sandbox routes one ID parser.

   • It covers GET and PATCH /sandboxes/{id}, POST /sandboxes/{id}:force-{action}, POST /sandboxes/{id}/commands and GET /debug/sandbox/{id}.
   • Each accepts sbx_<uuid>, a bare UUID, and an undashed UUID (hex in either case).
   • Each also accepts the sandbox pod name space-sbx-<undashed uuid>, with or without its -g<generation> suffix, and ignores surrounding whitespace.
   • Before this change, the sandbox routes took sbx_ followed by any spelling uuid parses. The debug route took a dashed, undashed, braced or urn:uuid: UUID without sbx_.
   • Any other form answers 400. That includes the braced and urn:uuid: spellings, SBX_ and the sandbox:<uuid> log tag. The console's parseSandboxRef also accepts those last two and normalizes them before sending, so it needs no change.
   • Operations, their idempotency fingerprints and the structured audit entries' sandbox targets always carry the canonical sbx_ ID, so one sandbox named two ways is one idempotency target. The request-level admin_audit_events.path still records the path as sent.

4. Adds GET /admin/api/v1/sandboxes/spaced-versions, with an optional org_id. It answers {counts: [{template, spaced_version, status, resource_type, count}]}: the console's tally of the full list, computed in one statement.

   • It counts every row whose status is not deleted, grouped by the fields the list response shows for that row:
     • template: the row's template, else the node's, else the template the node's inventory holds for the image it runs, else the pool's;
     • spaced_version: what the node last reported, null before a report;
     • status: the node's observed status, else the stored lifecycle. It is the same value as the response's status and lifecycle. A cleanup_required row whose node still reports started therefore counts as started, as it does in the console;
     • resource_type: sandbox, pool_slot or observed_local.
   • Without org_id it includes the node-reported rows with no sandbox row, as the unscoped list does.
   • The console's view can use it unchanged: tally status == "started" by template and spaced_version, and get its listed and node-reported totals from resource_type.
   • Unknown parameters answer 400.
   • The console currently samples the newest 5,000 list rows for this view, which misses long-lived sandboxes.

The filters, page statements and count live in the sandbox controller's new list_page::admin module, and the route keeps HTTP parsing only. The admin candidate statement is one fixed statement with bound nullable filters. A paged request composes it into the ECW list's existing page walk (list_page::page_rows_cte) and full-row select (org::scoped_page_sql, through sandbox_query), which interpolate SQL fragments. That composition predates this PR, which reuses it so both lists resolve rows and stop at the byte budget the same way. ListPage decides whether another page follows. The unpaged array runs a separate fixed statement with the same filters, over sandbox_query's shared columns and joins, as fetch_sandbox does.

The list handler moved from sandboxes.rs to routes/admin/sandbox_list.rs. That takes sandboxes.rs under the file-size limit and out of architecture-baseline.toml. The org-scoped list test from #1248 called the moved handler. Its two cases, an org_id list leaving out the node-reported rows and the unscoped list keeping them, are now part of the_list_pages_by_cursor_and_keeps_the_unpaged_array, which covers both the bare array and the paged walk.

The admin OpenAPI stub documents the new parameters and their semantics, the count route, GET /sandboxes/{sandbox_id} and the ID forms.

pplx-space-api gains no production dependency on pplx-space-placement, which main's layer rules forbid. The lifecycle filter validates against an API-local list of the stored lifecycles, the SandboxLifecycle values plus the deleted and cleanup_required tombstones, with the same normalization. A unit test pins that list to StoredSandboxLifecycle::all() through the existing test-support dev-dependency.

Cost and indexes

No migration. Measured on a migrated test database holding 300,000 synthetic sandboxes, 205,000 observed rows, 5,000 rowless pool slots, and a template inventory on 50 nodes:

• The count: about 0.5 s unscoped (305,000 rows counted into 175 buckets: a merge full join over both primary keys, plus a memoized inventory lookup), and about 90 ms for one org.
• A page of 1,000: 35–42 ms with no filter or with org_id, and 25–35 ms for a prefix. A page of 5,000 takes 216 ms and stops at the byte budget after 4,072 rows.
• The legacy unpaged array: 5,000 rows take 105–110 ms, against 103–113 ms for main's statement on the same data set.

Earlier EXPLAIN ANALYZE runs of the filter predicates on the same data set:

• Served by walking idx_sandboxes_created_at in list order: no filter, cursor, org_id, lifecycle, user_id, created_after/created_before, and a common template.
• Scan:
  • a rare template;
  • prefix: its name half has no index, and the OR keeps the ID half off the primary key;

An index for template or name would serve one filter each and add write cost to the busiest table. These timings seemed acceptable for an operator route, so this PR adds none. If one is wanted, it can land as its own migration first.

This unblocks the console work that depends on it:

• paging the sandbox list instead of sampling 5,000 rows;
• moving its filters server-side;
• one ID box for lookup across the sandbox and debug views;
• reading the spaced-version rollout from the server.

Validation

• Rebased onto main at c5fdf0c (after api: expose node, template build, quarantine and snapshot-storage state to the admin console #1307 merged). At this head, cargo nextest run -p pplx-space-api --all-features: 2,194 pass, 1 skipped (the profile's standing skip). api: expose node, template build, quarantine and snapshot-storage state to the admin console #1307's debug-route test now passes the sbx_<uuid> string the route takes.
• Postgres-backed controller tests (controllers::sandbox::list_page::admin::tests):
  • paging returns every row exactly once, in order, with rows sharing a created_at and rows created between pages;
  • the first unpaged page equals the pre-change statement's rows;
  • a table of each filter, alone and combined;
  • the ID-prefix range;
  • the count against the console's tally of the list's own projection (list_responses over every listed row). The seeds include an inventory-resolved template, a node template, a cleanup_required row and a stored-deleted row whose nodes still report them started (both counted, as in the console), deleted rows, other orgs, a rowless pool slot and a rowless observed-local row.
• Route tests through the real admin router with session auth (routes::admin::sandbox_list::tests):
  • the bare array without cursor, and the cursor walk with it, for an org_id list and the unscoped one, with next_cursor: null on the last page;
  • JSON 400s for every malformed parameter, typed ones included, on the list and the count;
  • an unknown parameter, now including node_id, refused with cursor and ignored without it;
  • created_before inside a microsecond;
  • 403 for a non-admin;
  • the count's shape;
  • every ID form on all five sandbox routes, with an unknown form answering 400 on each.
• routes::admin::sandbox_id::tests covers the accepted and refused forms, plus the pod builder's own names under the kubernetes feature.
• Mutation checks. Each of these fails the named test:
  • dropping the inventory template from the count, a tombstone pre-filter that drops stored-deleted rows, or the created_before rounding;
  • an inclusive keyset comparison;
  • keeping the node-reported rows under filters;
  • the sbx_-only parser on the commands route.
• cargo clippy -p pplx-space-api --all-targets --all-features -- -D warnings and cargo fmt are clean at this head.
• cargo arch at this head against main: no required findings, as on main, and nothing added:
  • the routes/admin/sandboxes.rs growth finding goes, with its baseline entry;
  • routes/admin/docs.rs, already over the file-size limit on main (1,018 code lines), grows to 1,054 with this route documentation.
• Kind (infra/space/kind/scripts/up.sh, Kubernetes substrate), at a12ff590, before the rebase. The rebase changed only a test call site and the lifecycle filter's parse type. Driven through /admin/api/v1 with synthetic orgs, users, seed sandboxes and a two-slot warm pool. 43 of 43 checks pass:
  • the org's list paged in 3 pages, each sandbox once, newest first, and the first page equals the bare array;
  • each remaining filter, walked page by page, matched the full list filtered client-side;
  • node_id with cursor answers 400 as an unknown parameter. Without cursor it is ignored, and the bare array equals the unfiltered one;
  • all seven ID forms resolved on GET /sandboxes/{id} and GET /debug/sandbox/{id}, including the live pod name;
  • malformed typed parameters answered JSON 400 on the list and the count;
  • the count, unscoped and for one org, equalled the console's tally of the full cursor walk. The unscoped walk was 10 rows, 2 of them the warm pool's rowless slots, which Kubernetes reports as observed_local.
  • Fixtures and the pool policy were removed and the cluster torn down.

Severity and risk

Severity: low. Nothing behaves incorrectly today, but operators could not page or filter the sandbox list server-side, count spaced versions beyond 5,000 rows, or look a sandbox up by the identifiers other tools print.

Risk:

• Existing callers: the unpaged list keeps its shape and rows, the new parameters are optional, and the count is a new route.
• The unpaged list runs the same filters through a plain statement, at main's speed: 105–110 ms for 5,000 rows, against 103–113 ms.
• The ID parser now refuses spellings no SPACE tool prints: braced and urn:uuid: UUIDs, on the debug route and after sbx_.
• The debug route's 400 for a bad ID is now JSON {"error": "invalid_sandbox_id"}, instead of axum's plain-text path rejection.
• Roll back by reverting the commits. Nothing is persisted in a new shape.

Rollout

The API, console and other services roll out separately. While old and new API replicas run together:

• On an API without this change, GET /admin/api/v1/sandboxes/spaced-versions falls into GET /sandboxes/{sandbox_id} and answers 400 "sandbox id must be an sbx_ prefixed UUID". The console should treat that 400 on this route as "not available" and fall back to its sampled tally.
• An older API ignores the filters and cursor and answers the bare array, so it never applies a filter. The console should send filters only with cursor. A bare array in reply means paging, and with it filtering, is unavailable: fall back to client-side filtering of the sampled list.
• During a rolling deploy, a page after the first can reach an old replica and come back as a bare array of the newest rows. The console should treat a bare array on any page after the first as "paging unavailable", and restart from the first page or fall back, rather than append those rows.

Stack

Independent of the other admin API PRs. The node_id and assigned filters can follow once the Direct route has an owning read. Open #1302 adds admin routes (terminal sessions, snapshots, keys) that parse only sbx_ IDs. Whichever of #1302 and this PR merges second moves those routes onto routes::admin::sandbox_id::parse_admin_sandbox_id.

AI assistance: Claude Code drafted the code, tests and this description; a human reviews before merge.

[github-actions]

Thanks for opening this pull request! A SPACE maintainer will take a look.

CONTRIBUTING.md explains how we review outside contributions.

[the-gigi-pplx]

Review — 8 findings

High

1. crates/libs/api/src/routes/debug/sandbox.rs:71 — the PR no longer compiles once it's merged with current main.
   This PR changes get_sandbox_debug from Path<SandboxId> to Path<String>. PR api: expose node, template build, quarantine and snapshot-storage state to the admin console #1307, merged to main after this branch's base, added a test that still calls the handler with Path(sandbox_id), where sandbox_id: SandboxId (routes/debug/sandbox/tests.rs:238 on main). Git reports no textual conflict, so the break only shows in CI's merge build: common / Rust tests and rust_static / Rust clippy both fail with E0308 expected String, found SandboxId at that line. The branch is 16 commits behind main.
   Rebase, and change that call to Path(sandbox_id.to_string()), as this PR already does for the other call at line 110.

Low

2. crates/libs/api/src/routes/admin/sandbox_list.rs:146 — the created_after < created_before check runs after created_before has been rounded up to the microsecond, so it accepts some inverted ranges.
   With created_after=…T00:00:00.0000008Z and created_before=…T00:00:00.0000004Z, before rounds up to .000001, which is later than after. The request passes, and it returns an empty list instead of the 400 the OpenAPI text promises ("later than created_after"). Compare the parsed values before rounding.
3. crates/libs/api/src/routes/admin/sandbox_list.rs:320-336 — the comment this PR moves here (and points sandboxes::get_sandbox at) contradicts the source.
   It says fetch_sandbox_observed_states and fetch_observed_only_sandboxes erase the SQLSTATE into ControllerError::internal, and that this arm flattens every error to a 500. Both reads map errors through db_contention_error (controllers/sandbox/responses.rs:488, :578), which classifies lock contention (controllers/mod.rs:460). support::controller_read_error already honours is_lock_contention() (routes/admin/support.rs:57). The text predates this PR, but the move is a chance to delete or correct it rather than give it a second caller.
4. crates/libs/api/src/controllers/sandbox/list_page/admin.rs:54 — prefix counts as a filter, so an ID search never finds a rowless warm-pool slot or observed_local row.
   is_unfiltered() gates include_observed_only. The stated reason, that none of the filters can be applied to those rows, holds for org_id, user_id and the timestamps, but not for an ID prefix: those rows have IDs. An operator who copies a pool_slot ID from the unfiltered list and searches its first 8 hex digits gets nothing. This is fine if intended, but then the doc comment and the OpenAPI text should say it.
5. crates/libs/api/src/routes/admin/sandbox_list.rs:210 — timestamps that jiff and time accept but Postgres can't store pass validation and then fail in the database.
   Examples are created_after=-5000-01-01T00:00:00Z and a hand-built cursor with a very negative nanosecond value. Postgres rejects the bind with 22008 timestamp out of range, which isn't a contention SQLSTATE, so the handler answers 500 for malformed client input. Clamp to Postgres's range at parse time and answer 400. (Traced through the code, not run.)
6. crates/libs/api/src/routes/admin/sandbox_list.rs:162, :195 — on the unpaged path, an empty value for a new filter name now answers 400, although the docs say unknown parameters are ignored without cursor.
   lifecycle= splits to [""] and answers "unknown lifecycle". template= fails validate_component's empty check. An empty prefix= is treated as unset. A form that sends blank fields to the legacy array used to get the full list. Treating an empty value as unset for all three would make them consistent.
7. crates/libs/api/src/controllers/sandbox/list_page/admin.rs:126, :158 — the nine-line filter WHERE clause appears in both ADMIN_CANDIDATE_ROWS and ADMIN_LIST_SQL, and the nine .bind(...) calls are repeated in both branches of admin_sandbox_page.
   A new filter, or a fix to one predicate, has to land in four places. If one copy is missed, the paged and unpaged lists silently return different rows for the same query. One shared WHERE fragment formatted into both statements, plus one bind helper, would keep them in step.
8. crates/libs/api/src/controllers/sandbox/list_page/admin.rs:310 — the count's FULL JOIN reads every deleted tombstone the retention window keeps, and only drops them after the join and the per-row inventory LATERAL.
   The cost grows with tombstone retention, not with live sandboxes. If the console refreshes this view on a timer, consider excluding stored-deleted rows that have no observed row before the join. Stored-deleted rows whose node still reports them have to stay, as your seeds show.

Verified against a12ff59074a931b0e8fa60d6016353516bb695ef.

What I ran:

• Read the failing CI logs, and checked the main call site that causes finding 1.
• Ran the PR's new and touched test modules at this head (sandbox_list, sandbox_id, list_page::admin, debug::sandbox, admin::sandboxes): 31 run, 31 passed (with --features kubernetes).

What I traced by reading:

• The count's template and status against the list projection (sandbox_to_cached_response, observed_only_sandbox_response, SandboxStateInput::reported_lifecycle). They agree.
• The keyset comparison in page_rows_cte.
• The ID-prefix range construction.
• The created_before rounding against sqlx's microsecond truncation.
• That no other admin or debug route on main still parses sandbox IDs on its own.

Not done: a Kind run, or a rerun of the timing measurements.

[the-gigi-pplx]

Review — follow-up, 1 new finding

The head is still a12ff59074a931b0e8fa60d6016353516bb695ef, so my earlier review comment stands as written. main has moved 27 commits since this branch's base, and one of them adds a second blocker that the rebase will surface.

High

1. crates/libs/api/Cargo.toml — after a rebase, pplx-space-placement as a normal dependency of pplx-space-api is a required layers.store_edges finding.
   PR archcheck: add a layers rule for crate placement and dependency direction #1152 made pplx-space-placement a declared store whose only users are pplx-space-internal-api-server and pplx-space-placement-executor (ARCHITECTURE.toml:673 on main). On main, pplx-space-api uses it only under [dev-dependencies], which the rule allows. I rebased this branch onto main in a scratch copy. cargo arch with main's baseline then reports 110 findings (109 advisory, 1 required), and the required one is this edge. cargo nextest run -p pplx-space-archcheck --test repository fails repository_config_and_baseline_pass_every_required_rule on it, so common / Rust tests stays red after the compile fix too.
   The production uses are only the lifecycle filter's parse (routes/admin/sandbox_list.rs:195) and the count's Deleted bind (controllers/sandbox/list_page/admin.rs:327). There are two ways out, and each has a cost:
   • Add pplx-space-api to the store's users. That's one line, but it's a layers.policy_changed change for a maintainer to accept. It also opens the whole store crate to the API, not just the vocabulary.
   • Move the stored-lifecycle vocabulary, which is pure data next to SandboxLifecycle in pplx-space-common-core, into common-core. Then the API depends on no store. The move touches the test imports that use it today. Keeping a second copy of the vocabulary in the API instead would need a test tying it to the original.

Not a finding

• The red CodeQL check (17 "cleartext logging of sensitive information" alerts) flags only assertion and panic! messages in routes/admin/sandbox_list/tests.rs and routes/admin/sandbox_id/tests.rs that print synthetic sandbox IDs. Nothing in production code is flagged. CodeQL isn't a required check on main; space-pull-ci and license/cla are.

What I ran, on the rebased scratch copy with Path(sandbox_id.to_string()) at routes/debug/sandbox/tests.rs:238

• cargo clippy -p pplx-space-api --features kubernetes --all-targets --locked -- -D warnings: clean. Those two changes are the whole compile fix.
• The new and touched test modules (sandbox_list, sandbox_id, list_page::admin, debug::sandbox, admin::sandboxes): 32 run, 32 passed, including the debug test main added.
• cargo arch with main's baseline and architecture files, plus the archcheck repository test, as above.

Not rerun: the full workspace test suite.

[the-gigi-pplx]

Review — re-review at 411c536b, both blockers fixed

Both High findings from my earlier comments are fixed:

• The merge with main compiles. I rebased this head onto current main in a scratch copy, and cargo clippy -p pplx-space-api --features kubernetes --all-targets --locked -- -D warnings is clean.
• No required layers.store_edges finding. pplx-space-placement is a dev-dependency again (crates/libs/api/Cargo.toml:114), and the lifecycle filter parses without the store. On the rebased tree, cargo arch with main's baseline and architecture files reports 112 findings (112 advisory, 0 required), and cargo nextest run -p pplx-space-archcheck --test repository passes 5 of 5.

The Lows from my first comment are all still present. None blocks; take or leave them:

• routes/admin/sandbox_list.rs:145 compares created_after against the rounded-up created_before, so a reversed range inside one microsecond answers 200 [] rather than 400.
• routes/admin/sandbox_list.rs:206 accepts timestamps outside Postgres's range, which then answer 500 rather than 400. A hand-built cursor reaches the same path.
• The moved comment at routes/admin/sandbox_list.rs:319 still says the projection reads erase the SQLSTATE, and routes/admin/sandboxes.rs:94 points to it ("the reads behind it do not classify yet either"). Both reads go through db_contention_error, and support::controller_read_error honours contention.
• prefix still turns off the node-reported rows (controllers/sandbox/list_page/admin.rs:95), so an ID search can't find a rowless pool slot.
• Empty lifecycle= and template= still answer 400 on the unpaged path.
• The filter WHERE and its binds are still written once per statement, and the count still reads every retained tombstone before discarding it.

Verified against 411c536b4901c5cabfa2c30f40d27a5f1ff3745c, rebased locally onto main. I ran the clippy, cargo arch and archcheck repository-test commands above on that tree. I didn't rerun the API test suite; CI is green at this head.

[the-gigi-pplx]

Approved. Both earlier blockers are fixed; the remaining findings in the review comment above are non-blocking.

[ac-pplx]

Thanks. Merging this as approved; the lows go into #1385, which edits the same files, as separate commits:

• the inverted range check runs on the parsed values, before created_before is rounded;
• timestamps outside Postgres's range, including in a cursor, answer 400 instead of 500;
• the moved comment about erased SQLSTATEs is corrected, along with the pointer in sandboxes.rs;
• empty lifecycle=, template=, node_id= and assigned= count as unset, so the unpaged path stays as lenient as before;
• the docs and OpenAPI say that prefix leaves out the node-reported rows;
• the count drops stored-deleted rows with no observed row before the join.

One I'll leave as is: the duplicated filter WHERE. The repository's data-store rules ask that every statement stay syntactically whole, with no shared or interpolated predicate fragments, so the two statements each spell out the filter. A test that runs every filter through both the paged and unpaged statements and compares their rows guards against the copies drifting. I'll add that test in #1385 if the existing ones don't already cover each filter.