Skip to content

Commit 33bbda3

Browse files
authored
refactor(persistence): adopt continuation-token pagination (#3249)
* refactor(persistence): adopt continuation-token pagination Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(pagination): address continuation review findings Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(tui): recover completed list refreshes Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(pagination): address review scalability findings Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(pagination): repair branch validation Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(go): use page size in template example Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent ddc8bba commit 33bbda3

84 files changed

Lines changed: 3544 additions & 1573 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.agents/skills/create-github-pr/SKILL.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,17 +187,17 @@ gh pr create \
187187
--body "$(cat <<'EOF'
188188
## Summary
189189
190-
Add `--limit` and `--offset` flags to `openshell sandbox list` for pagination.
190+
Add `--page-size` and `--page-token` flags to `openshell sandbox list` for continuation-token pagination.
191191
192192
## Related Issue
193193
194194
Closes #456
195195
196196
## Changes
197197
198-
- Added `offset` and `limit` query parameters to the sandbox list API call
199-
- Default limit is 20, max is 100
200-
- Response includes `total_count` field
198+
- Added `page_size` and `page_token` fields to the sandbox list API call
199+
- Default page size is 100, max is 1,000
200+
- Structured responses include `next_page_token`
201201
202202
## Testing
203203

‎.agents/skills/tui-development/SKILL.md‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,15 @@ Phase 1: GetSandboxLogs → 500 initial lines → send via Event::LogLines
170170
Phase 2: WatchSandbox(follow_logs: true) → live tail → send via Event::LogLines
171171
```
172172

173-
**Sandboxes**: Fetched via `ListSandboxes` on a 2-second tick, scoped to the current workspace (or all workspaces).
173+
**Sandboxes**: Fetched via `ListSandboxes` in a background collection-refresh task scheduled from the 2-second tick, scoped to the current workspace (or all workspaces). Follow `next_page_token` until empty so the dashboard reflects the complete collection.
174174

175-
**Providers**: Fetched via `ListProviders` on each tick. Provider profiles are fetched per-workspace via `ListProviderProfiles` and cached in a `ProviderProfileCache` keyed by `(workspace, profile_id)`.
175+
**Providers**: Fetched via `ListProviders` in the background collection-refresh task. Provider profiles are fetched per-workspace via `ListProviderProfiles` and cached in a `ProviderProfileCache` keyed by `(workspace, profile_id)`. Follow each list RPC's `next_page_token` until empty.
176176

177177
**Settings**: Global settings are fetched via `GetGatewayConfig` on each tick. Sandbox settings are fetched alongside the sandbox policy via `GetSandboxConfig` and refreshed on each tick when viewing a sandbox.
178178

179-
**Workspaces**: The workspace list is fetched via `ListWorkspaces` on each tick.
179+
**Workspaces**: The workspace list is fetched via `ListWorkspaces` in the background collection-refresh task, following `next_page_token` until empty.
180+
181+
Only one collection-refresh task may run at a time. Workspace and gateway changes abort the active task, and refresh results carry their gateway/workspace context so stale results are discarded.
180182

181183
### Never block the event loop
182184

@@ -411,9 +413,9 @@ All actions are accessible via keyboard shortcuts displayed in the nav bar. The
411413
| File | Purpose |
412414
| --- | --- |
413415
| `crates/openshell-tui/Cargo.toml` | Crate manifest — dependencies on `openshell-core`, `openshell-bootstrap`, `ratatui`, `crossterm`, `tonic`, `tokio` |
414-
| `crates/openshell-tui/src/lib.rs` | Entry point. Event loop, gRPC calls (`refresh_data`, `refresh_providers`, `refresh_global_settings`, `refresh_workspaces`, `refresh_sandboxes`, `spawn_log_stream`, `handle_sandbox_delete`), gateway switching, mTLS channel building, provider CRUD spawners, settings CRUD spawners, draft approval spawners |
416+
| `crates/openshell-tui/src/lib.rs` | Entry point. Event loop, background collection refresh (`spawn_list_refresh`), gRPC calls (`refresh_global_settings`, `spawn_log_stream`, `handle_sandbox_delete`), gateway switching, mTLS channel building, provider CRUD spawners, settings CRUD spawners, draft approval spawners |
415417
| `crates/openshell-tui/src/app.rs` | `App` state struct, `Screen`/`Focus`/`InputMode`/`LogSourceFilter`/`MiddlePaneTab`/`SandboxPolicyTab` enums, `LogLine`/`GatewayEntry`/`GlobalSettingEntry`/`SandboxSettingEntry`/`ProviderListEntry`/`ProviderDetailView` structs, create sandbox/provider form state, all key handling logic |
416-
| `crates/openshell-tui/src/event.rs` | `Event` enum (`Key`, `Mouse`, `Tick`, `Redraw`, `Resize`, `LogLines`, `CreateResult`, `ProviderCreateResult`, `ProviderDetailFetched`, `ProviderUpdateResult`, `ProviderDeleteResult`, `DraftActionResult`, `GlobalSettingsFetched`, `GlobalSettingSetResult`, `GlobalSettingDeleteResult`, `SandboxSettingSetResult`, `SandboxSettingDeleteResult`, `ForwardWarnings`), `EventHandler` with mpsc channels and crossterm polling |
418+
| `crates/openshell-tui/src/event.rs` | `Event` enum (`Key`, `Mouse`, `Tick`, `Redraw`, `Resize`, `LogLines`, `ListRefreshCompleted`, `CreateResult`, `ProviderCreateResult`, `ProviderDetailFetched`, `ProviderUpdateResult`, `ProviderDeleteResult`, `DraftActionResult`, `GlobalSettingsFetched`, `GlobalSettingSetResult`, `GlobalSettingDeleteResult`, `SandboxSettingSetResult`, `SandboxSettingDeleteResult`, `ForwardWarnings`), `EventHandler` with mpsc channels and crossterm polling |
417419
| `crates/openshell-tui/src/theme.rs` | `colors` module (NVIDIA_GREEN, EVERGLADE, BG, FG) and `styles` module (all `Style` constants) |
418420
| `crates/openshell-tui/src/clipboard.rs` | Clipboard copy support for log lines |
419421
| `crates/openshell-tui/src/ui/mod.rs` | Top-level `draw()` dispatcher, `draw_title_bar` (with workspace display), `draw_nav_bar`, `draw_command_bar`, screen routing, shared setting-edit overlay, modal helpers |
@@ -502,12 +504,15 @@ use openshell_core::proto::{
502504
`Some(all_workspaces_selector())`; do not use that marker on other requests.
503505
- `GetSandboxLogsRequest` fields: `sandbox_id`, `lines` (u32), `since_ms` (i64),
504506
`sources` (Vec<String>), `min_level` (String), `workspace_scope`.
505-
- `ListSandboxesRequest` fields: `limit` (u32), `offset` (u32),
507+
- `ListSandboxesRequest` fields: `page_size` (i32), `page_token` (String),
506508
`label_selector` (String), `workspace_scope`.
507-
- `ListProvidersRequest` fields: `limit` (u32), `offset` (u32),
509+
- `ListProvidersRequest` fields: `page_size` (i32), `page_token` (String),
508510
`workspace_scope`.
509-
- `ListWorkspacesRequest` fields: `limit` (u32), `offset` (u32),
511+
- `ListWorkspacesRequest` fields: `page_size` (i32), `page_token` (String),
510512
`label_selector` (String).
513+
- Paginated list responses return `next_page_token`. Continue with the same
514+
request parameters and that token until it is empty; changing filters or
515+
scope invalidates the token.
511516
- `UpdateConfigRequest` fields include `name` (String, sandbox name or empty for
512517
global), `setting_key`, `setting_value`, `delete_setting` (bool), `global`
513518
(bool), and `workspace_scope`. Sandbox-scoped updates require a named selector;
@@ -543,21 +548,21 @@ The connect timeout for gateway switching is 10 seconds with HTTP/2 keepalive at
543548
4. On success:
544549
- `app.client` is replaced with a new intercepted client
545550
- `reset_sandbox_state()` clears all sandbox/log/draft/policy data
546-
- `refresh_data()` runs the full capability refresh sequence: `refresh_health` → `refresh_global_settings` → `refresh_workspaces` → `refresh_providers` → `refresh_sandboxes`
551+
- health and global settings are refreshed, then `spawn_list_refresh()` starts the cancellable workspace/provider/sandbox refresh task
547552
5. On failure: `status_text` shows the error
548553

549554
### Initial startup lifecycle
550555

551556
On launch, before the event loop starts:
552557

553558
1. `refresh_gateway_list()` — discover gateways from disk
554-
2. `refresh_data()` — full refresh (health, global settings, workspaces, providers, sandboxes)
559+
2. Refresh health and global settings, then start `spawn_list_refresh()` for workspaces, providers, and sandboxes
555560

556561
### Workspace switching lifecycle
557562

558563
1. User presses `[w]` on the providers or sandboxes panel → `cycle_workspace()` advances through discovered workspace names, then "all"
559564
2. `pending_workspace_refresh = true` is set, cursor indices are reset
560-
3. Event loop calls `refresh_providers()` and `refresh_sandboxes()` with the new workspace scope
565+
3. Event loop cancels any in-flight collection refresh and starts `spawn_list_refresh()` with the new workspace scope
561566

562567
### Settings CRUD lifecycle (global and sandbox)
563568

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎architecture/gateway.md‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ Compute-driver, credential-driver, gateway-interceptor, and
329329
supervisor-middleware services are compiled contracts for internal extension
330330
boundaries, not public gateway RPCs. The current public inventory has 74
331331
methods, 278 messages, and 12 enums
332-
(`c95ae90962c10fb28747db2b645adf4044562a3d208d84dfe4699d677e4364ee`).
332+
(`0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46`).
333333

334334
Storage-only messages live in the private, versioned
335335
`openshell.storage.v1` package under `crates/openshell-server/proto`. The server
@@ -529,11 +529,33 @@ modes:
529529
`UpdateProvider`, `UpdateProviderProfiles`, and `UpdateConfig` (policy
530530
backfill and sandbox annotation updates).
531531

532-
**Lists.** The `list_messages` and `list_messages_with_selector` helpers decode
533-
protobuf payloads from list results and hydrate `resource_version` from the
534-
authoritative database column into each decoded message, mirroring the
535-
`get_message` pattern. This ensures list responses carry correct versions
536-
without requiring callers to manually hydrate each record.
532+
**Lists.** Public list RPCs follow AIP-158: requests carry direct `page_size`
533+
and `page_token` fields, and responses carry `next_page_token`. The gateway
534+
clamps page sizes to 1,000 and returns opaque base64url continuation tokens.
535+
Tokens bind the RPC and every request parameter except `page_size`, contain no
536+
authorization grant, and use immutable keyset cursors rather than database
537+
offsets. Each page repeats normal authentication and authorization. Pagination
538+
is weakly consistent under concurrent writes and deletes; it does not provide a
539+
historical snapshot.
540+
541+
The token wire format is a private shared protobuf used only by the gateway.
542+
Public request and response messages repeat the standard AIP fields directly
543+
instead of wrapping them in a shared pagination message.
544+
545+
The CLI returns paginated JSON and YAML as response-shaped envelopes containing
546+
the resource collection and `next_page_token`; table output reports a non-empty
547+
token on stderr. The TUI traverses complete workspace, provider, profile, and
548+
sandbox collections in one cancellable background refresh task, never overlaps
549+
periodic list refreshes, and discards results after a gateway or workspace
550+
change.
551+
552+
Persistence distinguishes one-page operations from exhaustive scans.
553+
`list_object_page` and `list_message_page` return one keyset page and its next
554+
cursor. `collect_records` and `collect_messages` exhaust those pages, fail on
555+
database or protobuf decode errors, and hydrate `resource_version` from the
556+
authoritative database column. Internal callers that require every matching
557+
record use the exhaustive helpers; bounded lookups continue to use page-level
558+
methods.
537559

538560
**Deletes.** Delete operations are not yet CAS-protected -- the delete request
539561
protos do not carry `expected_resource_version`. A `delete_if` primitive exists

‎crates/openshell-cli/src/commands/provider.rs‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -298,18 +298,18 @@ pub async fn ensure_required_providers(
298298
let mut known_names: HashSet<String> = HashSet::new();
299299
let mut type_to_name: HashMap<String, String> = HashMap::new();
300300
{
301-
let mut offset = 0_u32;
302-
let limit = 100_u32;
301+
let mut page_token = String::new();
303302
loop {
304303
let response = client
305304
.list_providers(ListProvidersRequest {
306-
limit,
307-
offset,
305+
page_size: 100,
306+
page_token,
308307
workspace_scope: Some(openshell_core::proto::workspace_selector(workspace)),
309308
})
310309
.await
311310
.into_diagnostic()?;
312-
let providers = response.into_inner().providers;
311+
let response = response.into_inner();
312+
let providers = response.providers;
313313
for provider in &providers {
314314
known_names.insert(provider.object_name().to_string());
315315
if !provider.r#type.is_empty() {
@@ -319,10 +319,10 @@ pub async fn ensure_required_providers(
319319
.or_insert_with(|| provider.object_name().to_string());
320320
}
321321
}
322-
if providers.len() < limit as usize {
322+
if response.next_page_token.is_empty() {
323323
break;
324324
}
325-
offset = offset.saturating_add(limit);
325+
page_token = response.next_page_token;
326326
}
327327
}
328328

@@ -1325,8 +1325,8 @@ fn provider_credential_keys(provider: &Provider) -> Vec<String> {
13251325
#[allow(clippy::too_many_arguments)]
13261326
pub async fn provider_list(
13271327
server: &str,
1328-
limit: u32,
1329-
offset: u32,
1328+
page_size: i32,
1329+
page_token: &str,
13301330
names_only: bool,
13311331
output: &str,
13321332
workspace: &str,
@@ -1336,8 +1336,8 @@ pub async fn provider_list(
13361336
let mut client = grpc_client(server, tls).await?;
13371337
let response = client
13381338
.list_providers(ListProvidersRequest {
1339-
limit,
1340-
offset,
1339+
page_size,
1340+
page_token: page_token.to_string(),
13411341
workspace_scope: Some(if all_workspaces {
13421342
openshell_core::proto::all_workspaces_selector()
13431343
} else {
@@ -1346,12 +1346,21 @@ pub async fn provider_list(
13461346
})
13471347
.await
13481348
.into_diagnostic()?;
1349-
let providers = response.into_inner().providers;
1349+
let response = response.into_inner();
1350+
let next_page_token = response.next_page_token;
1351+
let providers = response.providers;
13501352

13511353
// Handle structured output formats (json, yaml)
1352-
if crate::output::print_output_collection(output, &providers, provider_to_json)? {
1354+
if crate::output::print_paginated_output_collection(
1355+
output,
1356+
"providers",
1357+
&providers,
1358+
&next_page_token,
1359+
provider_to_json,
1360+
)? {
13531361
return Ok(());
13541362
}
1363+
crate::output::print_next_page_token(&next_page_token);
13551364

13561365
if providers.is_empty() {
13571366
if !names_only {
@@ -1444,15 +1453,24 @@ pub async fn provider_list_profiles(
14441453
tls: &TlsOptions,
14451454
) -> Result<()> {
14461455
let mut client = grpc_client(server, tls).await?;
1447-
let response = client
1448-
.list_provider_profiles(ListProviderProfilesRequest {
1449-
limit: 100,
1450-
offset: 0,
1451-
workspace: workspace.to_string(),
1452-
})
1453-
.await
1454-
.into_diagnostic()?;
1455-
let mut profiles = response.into_inner().profiles;
1456+
let mut page_token = String::new();
1457+
let mut profiles = Vec::new();
1458+
loop {
1459+
let response = client
1460+
.list_provider_profiles(ListProviderProfilesRequest {
1461+
page_size: 100,
1462+
page_token,
1463+
workspace: workspace.to_string(),
1464+
})
1465+
.await
1466+
.into_diagnostic()?
1467+
.into_inner();
1468+
profiles.extend(response.profiles);
1469+
if response.next_page_token.is_empty() {
1470+
break;
1471+
}
1472+
page_token = response.next_page_token;
1473+
}
14561474
profiles.sort_by(|left, right| {
14571475
left.category
14581476
.cmp(&right.category)

‎crates/openshell-cli/src/completers.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ pub fn complete_sandbox_names(_prefix: &OsStr) -> Vec<CompletionCandidate> {
3636
let mut client = completion_grpc_client(&endpoint, &gateway_name).await?;
3737
let response = client
3838
.list_sandboxes(ListSandboxesRequest {
39-
limit: 200,
40-
offset: 0,
39+
page_size: 200,
40+
page_token: String::new(),
4141
label_selector: String::new(),
4242
workspace_scope: Some(openshell_core::proto::workspace_selector(
4343
workspace_from_args(),
@@ -63,8 +63,8 @@ pub fn complete_provider_names(_prefix: &OsStr) -> Vec<CompletionCandidate> {
6363
let mut client = completion_grpc_client(&endpoint, &gateway_name).await?;
6464
let response = client
6565
.list_providers(ListProvidersRequest {
66-
limit: 200,
67-
offset: 0,
66+
page_size: 200,
67+
page_token: String::new(),
6868
workspace_scope: Some(openshell_core::proto::workspace_selector(
6969
workspace_from_args(),
7070
)),
@@ -89,8 +89,8 @@ pub fn complete_workspace_names(_prefix: &OsStr) -> Vec<CompletionCandidate> {
8989
let mut client = completion_grpc_client(&endpoint, &gateway_name).await?;
9090
let response = client
9191
.list_workspaces(ListWorkspacesRequest {
92-
limit: 200,
93-
offset: 0,
92+
page_size: 200,
93+
page_token: String::new(),
9494
label_selector: String::new(),
9595
})
9696
.await

0 commit comments

Comments
 (0)