diff --git a/architecture/gateway.md b/architecture/gateway.md index 8fc28dfb21..100aabd0c4 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -544,14 +544,23 @@ modes: backfill and sandbox annotation updates). **Lists.** Public list RPCs follow AIP-158: requests carry direct `page_size` -and `page_token` fields, and responses carry `next_page_token`. The gateway -clamps page sizes to 1,000 and returns opaque base64url continuation tokens. +and `page_token` fields, and responses carry `next_page_token`. The sole +exception is `ListSandboxProviders`: a sandbox can have at most 32 attached +providers, so the gateway returns its complete bounded result in one response +and does not expose pagination fields. The gateway clamps page sizes to 1,000 +and returns opaque base64url continuation tokens. Tokens bind the RPC and every request parameter except `page_size`, contain no authorization grant, and use immutable keyset cursors rather than database offsets. Each page repeats normal authentication and authorization. Pagination is weakly consistent under concurrent writes and deletes; it does not provide a historical snapshot. +Object-backed lists (`ListSandboxTemplates`, `ListSandboxes`, `ListServices`, +`ListProviders`, `ListWorkspaces`, and `ListWorkspaceMembers`) sort ascending +by `(created_at_ms, name, workspace, id)`. `ListProviderProfiles` sorts +ascending by `(id, scope)`, and `ListSandboxPolicies` sorts by descending policy +version. `ListSandboxProviders` preserves the sandbox's attachment order. + The token wire format is a private shared protobuf used only by the gateway. Public request and response messages repeat the standard AIP fields directly instead of wrapping them in a shared pagination message. @@ -567,6 +576,16 @@ Curated Rust, Python, Go, and TypeScript SDK list methods return lazy pagers. Advancing a pager issues one list RPC and exposes its continuation token; explicit `list_all` helpers are the only curated APIs that exhaust a collection. +**Migration.** This pagination contract is a breaking replacement for the +former `limit`/`offset` list APIs. Protocol clients must send `page_size` and +resume only with the returned `next_page_token`; an empty token is the sole +completion signal. SDK callers that need every item must use the explicit +full-iteration helper (for example, Rust's `list_all_sandboxes`) rather than +awaiting `list_sandboxes`, which now returns one-page `Pager` state. Callers +that need one page should advance that pager once and retain its token. Internal +full scans use the reusable iteration helpers from the persistence pagination +audit rather than manually advancing offsets. + Persistence distinguishes one-page operations from exhaustive scans. `list_object_page` and `list_message_page` return one keyset page and its next cursor. `collect_records` and `collect_messages` exhaust those pages, fail on diff --git a/crates/openshell-sdk/README.md b/crates/openshell-sdk/README.md index 3adbcba375..e2b8a39973 100644 --- a/crates/openshell-sdk/README.md +++ b/crates/openshell-sdk/README.md @@ -60,9 +60,11 @@ workspace. Cross-workspace listing uses the separate `*_all_workspaces` methods and requires Platform Admin access. Curated `list_*` methods return a lazy `Pager`. Each `next_page()` call -issues at most one RPC and returns a `Page` with its opaque continuation -token. The explicit `list_all_*` conveniences exhaust that pager; `page_size` -always controls one gateway request, and `page_token` resumes a saved traversal. +fetches one logical page and returns a `Page` with its opaque continuation +token. OIDC authentication can retry that page once after an `Unauthenticated` +response. The explicit `list_all_*` conveniences exhaust that pager; +`page_size` always controls one gateway request, and `page_token` resumes a +saved traversal. ```rust let mut pages = client.list_sandboxes(ListOptions { diff --git a/crates/openshell-sdk/src/pagination.rs b/crates/openshell-sdk/src/pagination.rs index 8f35d03dab..94943436fd 100644 --- a/crates/openshell-sdk/src/pagination.rs +++ b/crates/openshell-sdk/src/pagination.rs @@ -22,7 +22,9 @@ pub struct Page { /// A lazy, single-pass iterator over response pages. /// /// Constructed by curated `list_*` methods. No RPC is issued until -/// [`Pager::next_page`] is called, and each call fetches at most one page. +/// [`Pager::next_page`] is called, and each call fetches one logical page. +/// A refreshed OIDC credential may retry that page once after an +/// `Unauthenticated` response. pub struct Pager { fetch: PageFetcher, next_page_token: Option, diff --git a/crates/openshell-server/src/pagination.rs b/crates/openshell-server/src/pagination.rs index 66dd1d092e..4ae9f5b769 100644 --- a/crates/openshell-server/src/pagination.rs +++ b/crates/openshell-server/src/pagination.rs @@ -165,6 +165,78 @@ fn fingerprint(parameters: &[&str]) -> Vec { mod tests { use super::*; + fn message_body<'a>(proto: &'a str, message: &str) -> &'a str { + let marker = format!("message {message} {{"); + let start = proto + .find(&marker) + .unwrap_or_else(|| panic!("{message} message must exist")) + + marker.len(); + proto[start..] + .split_once("\n}") + .unwrap_or_else(|| panic!("{message} message must close")) + .0 + } + + fn message_documentation<'a>(proto: &'a str, message: &str) -> &'a str { + let marker = format!("message {message} {{"); + let message_start = proto + .find(&marker) + .unwrap_or_else(|| panic!("{message} message must exist")); + let documentation_start = proto[..message_start] + .rfind("\n\n") + .map_or(0, |boundary| boundary + 2); + &proto[documentation_start..message_start] + } + + #[test] + fn every_public_list_rpc_is_paginated_or_explicitly_bounded() { + let proto = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../proto/openshell.proto" + )); + let normalized = proto.lines().map(str::trim).collect::>().join(" "); + let mut remaining = normalized.as_str(); + + while let Some(start) = remaining.find("rpc List") { + remaining = &remaining[start + "rpc ".len()..]; + let (method, after_request) = remaining + .split_once('(') + .expect("List RPC must declare a request type"); + let (_, after_returns) = after_request + .split_once("returns (") + .expect("List RPC must declare a response type"); + let (response, after_response) = after_returns + .split_once(')') + .expect("List RPC response type must close"); + let request = format!("{method}Request"); + + if method == "ListSandboxProviders" { + assert!( + message_documentation(proto, &request) + .contains("bounded list intentionally has no pagination"), + "{request} must document why it has no pagination" + ); + assert!(message_body(proto, response).contains("complete bounded set")); + } else { + let request_body = message_body(proto, &request); + assert!( + request_body.contains("int32 page_size"), + "{request} must declare page_size" + ); + assert!( + request_body.contains("string page_token"), + "{request} must declare page_token" + ); + assert!( + message_body(proto, response).contains("string next_page_token"), + "{response} must declare next_page_token" + ); + } + + remaining = after_response; + } + } + #[test] fn page_size_defaults_clamps_and_rejects_negative_values() { assert_eq!( diff --git a/proto/openshell.proto b/proto/openshell.proto index def6d7c473..d3dfcf98d1 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -1244,7 +1244,9 @@ message ListSandboxesRequest { openshell.datamodel.v1.WorkspaceSelector workspace_scope = 6; } -// List providers attached to a sandbox request. +// List providers attached to a sandbox request. A sandbox can have at most 32 +// attached providers, so this bounded list intentionally has no pagination +// fields. message ListSandboxProvidersRequest { reserved 2; reserved "workspace"; @@ -1332,6 +1334,7 @@ message ListSandboxesResponse { // List providers attached to a sandbox response. message ListSandboxProvidersResponse { + // The complete bounded set of providers attached to the sandbox. repeated openshell.datamodel.v1.Provider providers = 1; } diff --git a/python/openshell/sandbox.py b/python/openshell/sandbox.py index 931fa1e2ab..ac3511b520 100644 --- a/python/openshell/sandbox.py +++ b/python/openshell/sandbox.py @@ -47,11 +47,12 @@ class Page(Generic[T]): class Pager(Generic[T]): - """Lazy, single-pass iterator that fetches one RPC page per advance.""" + """Lazy, single-pass iterator over the continuation-token contract.""" def __init__(self, fetch: Callable[[str], Page[T]], page_token: str = "") -> None: self._fetch = fetch self._page_token: str | None = page_token + self._consumed_page_tokens: set[str] = set() def __iter__(self) -> Pager[T]: return self @@ -59,8 +60,14 @@ def __iter__(self) -> Pager[T]: def __next__(self) -> Page[T]: if self._page_token is None: raise StopIteration - page = self._fetch(self._page_token) - self._page_token = page.next_page_token or None + page_token = self._page_token + page = self._fetch(page_token) + if page_token: + self._consumed_page_tokens.add(page_token) + next_page_token = page.next_page_token + if next_page_token and next_page_token in self._consumed_page_tokens: + raise SandboxError("pager received a repeated continuation token") + self._page_token = next_page_token or None return page def all(self) -> builtins.list[T]: diff --git a/python/openshell/sandbox_test.py b/python/openshell/sandbox_test.py index 0675bc7338..38fdc8517c 100644 --- a/python/openshell/sandbox_test.py +++ b/python/openshell/sandbox_test.py @@ -2610,6 +2610,16 @@ def fetch(token: str) -> Page[int]: assert tokens == ["resume", "resume"] +def test_pager_rejects_a_repeated_continuation_token() -> None: + pager = Pager( + lambda token: Page(items=[1], next_page_token=token), + page_token="resume", + ) + + with pytest.raises(SandboxError, match="repeated continuation token"): + next(pager) + + def test_list_ids_forwards_label_selector() -> None: stub = _FakeSandboxStub(listed=[_make_sandbox_proto("sandbox-1", "job-1")]) client = _client_with_fake_stub(stub) diff --git a/sdk/go/openshell/v1/pager.go b/sdk/go/openshell/v1/pager.go index 558fda4694..a6603c9882 100644 --- a/sdk/go/openshell/v1/pager.go +++ b/sdk/go/openshell/v1/pager.go @@ -16,17 +16,18 @@ type Page[T any] struct { type pageFetcher[T any] func(context.Context, string) (*Page[T], error) -// Pager lazily fetches one RPC page per call to NextPage. +// Pager lazily fetches pages from the continuation-token contract. // // A Pager is single-pass and must not be used concurrently. type Pager[T any] struct { - fetch pageFetcher[T] - nextPageToken *string + fetch pageFetcher[T] + nextPageToken *string + consumedTokens map[string]struct{} } // NewPager constructs a pager from an RPC page fetcher. func NewPager[T any](pageToken string, fetch func(context.Context, string) (*Page[T], error)) *Pager[T] { - return &Pager[T]{fetch: fetch, nextPageToken: &pageToken} + return &Pager[T]{fetch: fetch, nextPageToken: &pageToken, consumedTokens: make(map[string]struct{})} } func newPager[T any](pageToken string, fetch pageFetcher[T]) *Pager[T] { @@ -48,6 +49,12 @@ func (p *Pager[T]) NextPage(ctx context.Context) (*Page[T], error) { if page.Items == nil { page.Items = make([]T, 0) } + if *p.nextPageToken != "" { + p.consumedTokens[*p.nextPageToken] = struct{}{} + } + if _, seen := p.consumedTokens[page.NextPageToken]; page.NextPageToken != "" && seen { + return nil, errors.New("pager received a repeated continuation token") + } if page.NextPageToken == "" { p.nextPageToken = nil } else { diff --git a/sdk/go/openshell/v1/pager_test.go b/sdk/go/openshell/v1/pager_test.go index 55dc74eb28..063efc5ee8 100644 --- a/sdk/go/openshell/v1/pager_test.go +++ b/sdk/go/openshell/v1/pager_test.go @@ -58,6 +58,16 @@ func TestPagerAllRetriesTheCurrentTokenAfterError(t *testing.T) { assert.Equal(t, []int{1, 2}, items) } +func TestPagerRejectsRepeatedContinuationToken(t *testing.T) { + pager := NewPager("resume-token", func(_ context.Context, token string) (*Page[string], error) { + return &Page[string]{Items: []string{"first"}, NextPageToken: token}, nil + }) + + page, err := pager.NextPage(context.Background()) + assert.Nil(t, page) + assert.EqualError(t, err, "pager received a repeated continuation token") +} + func TestPagerNormalizesEmptyItems(t *testing.T) { pager := NewPager("", func(_ context.Context, _ string) (*Page[string], error) { return &Page[string]{}, nil diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index 735c17bc88..1568569168 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -3214,7 +3214,9 @@ func (x *ListSandboxesRequest) GetWorkspaceScope() *datamodelv1.WorkspaceSelecto return nil } -// List providers attached to a sandbox request. +// List providers attached to a sandbox request. A sandbox can have at most 32 +// attached providers, so this bounded list intentionally has no pagination +// fields. type ListSandboxProvidersRequest struct { state protoimpl.MessageState `protogen:"open.v1"` // Sandbox name (canonical lookup key). @@ -3687,7 +3689,8 @@ func (x *ListSandboxesResponse) GetNextPageToken() string { // List providers attached to a sandbox response. type ListSandboxProvidersResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` + state protoimpl.MessageState `protogen:"open.v1"` + // The complete bounded set of providers attached to the sandbox. Providers []*datamodelv1.Provider `protobuf:"bytes,1,rep,name=providers,proto3" json:"providers,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache