[PR #375] [MERGED] refactor(gateway): route from one table and share sandbox list cursor vectors #387

New issue

Closed

opened 2026-10-29 14:22:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:22:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/375
Author: @yingdi-shan
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @yingdi-shan

Base: main ← Head: refactor/gateway-route-table

📝 Commits (2)

ba27af9 refactor(gateway): route sandbox and build paths from one table
b7723e5 fix(gateway): match node API sandbox list cursors via shared vectors

📊 Changes

10 files changed (+645 additions, -194 deletions)

View changed files

📝 .github/workflows/services-ci.yml (+4 -0)
📝 services/gateway/internal/cluster_list.go (+14 -14)
📝 services/gateway/internal/cluster_list_test.go (+105 -3)
➕ services/gateway/internal/routes.go (+111 -0)
➕ services/gateway/internal/routes_test.go (+108 -0)
📝 services/gateway/internal/server.go (+21 -112)
📝 services/gateway/internal/server_test.go (+3 -64)
📝 services/go.mod (+1 -1)
📝 src/api/impls/pagination.rs (+108 -0)
➕ src/api/testdata/pagination.json (+170 -0)

📄 Description

What

Two commits, both in the gateway:

refactor(gateway): route sandbox and build paths from one table. The gateway's hand-written path parsers (isSandboxControlPlaneRequest, sandboxIDFromPath, templateBuildIDFromPath, v2TemplateBuildIDFromPath, isTemplateBuilderAllocation, isForkPath) become one table, pathRoutes in services/gateway/internal/routes.go. Each entry names a method, a path pattern, and how the owning node is picked. A new test reads src/api/openapi.yml and fails if an operation addressed by {sandboxID} or {buildID} is missing from the table, or if a table entry matches no operation.
fix(gateway): match node API sandbox list cursors via shared vectors. src/api/testdata/pagination.json holds cursor tokens to accept, reject and encode, plus pages to cut from a fixed set of sandboxes. The Rust cursor tests (src/api/impls/pagination.rs) and the gateway's cluster-list tests both read it. The gateway is fixed to match the node API.

Why

These came out of a review of why recent feature PRs touch 20–40 files.

Routing: every sandbox- or build-scoped endpoint needed edits to several Go path parsers. #338 (logs) and #347 (image builds) both did this. If an edit was missed, nothing failed: the request was routed to a scheduled node instead of the sandbox's node.
Pagination: the gateway re-implements the node API's v2 sandbox list cursor in Go (cluster_list.go), and nothing kept the two in step. #209 and #263 both had to change both sides. The new vectors show they had already drifted, as listed below.

N/A. Follow-up to the review behind #367.

Scope and non-goals

Routing is unchanged for every request that reaches path routing. Requests the gateway answers itself (lists, node detail, sandbox metrics list) are handled before path routing, as before.
The route table is hand-written Go and checked against the spec, not generated from it. Generating it would need an x- extension in openapi.yml; that is a possible follow-up.
Not included: image-build routes from #347. When #347 lands, they become table entries (ownerNewBuildConfirmed already covers allocations whose ID comes back in x-agentenv-build-id).

Design and behavior changes

Route owner kinds, which replace the separate parsers:

Kind	Picks the node by	Records an assignment
`ownerSandbox`	sandbox in `{id}`	no
`ownerSandboxFork`	source sandbox in `{id}`	children from the response
`ownerBuild`	build in `{id}`	no
`ownerBuildRead`	build in `{id}`; a build with no binding falls back to scheduling	no
`ownerNewBuild`	scheduling	`{id}`
`ownerNewBuildConfirmed`	scheduling	`x-agentenv-build-id`, which must equal `{id}`

Gateway behavior changes from the shared vectors:

No extra empty page. The gateway issued x-next-token whenever a page was exactly limit long. Clients then fetched an empty final page. Nodes issue a token only when more items remain, and the gateway now does the same.
Token format. The gateway wrote RFC3339Nano timestamps, which drop trailing zeros. Nodes always write nine fraction digits. The gateway now writes the same format. Tokens in the old format are still accepted.

Compatibility and operations

Public API or generated protocol: no schema change. Gateway list responses no longer include x-next-token on a final page that is exactly limit long.
Configuration or defaults: none.
Snapshot manifest, artifact layout, or storage format: none.
Upgrade and rollback: next tokens issued by the old gateway still parse.
Host requirements, permissions, ports, or dependencies: gopkg.in/yaml.v3 moves from indirect to direct in services/go.mod (test only; already in go.sum). Services CI now also runs when src/api/openapi.yml or src/api/testdata/pagination.json changes, since the gateway tests read both.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

make fmt                                            # clean
make clippy                                         # clean
cargo test -p agentenv -p envd -p linux-cap --lib   # agentenv: 963 passed, 4 ignored; envd: 3 passed
make -C services fmt-check vet                      # clean
make -C services build                              # ok
go test ./gateway/... ./scheduler/... ./shared/... ./api/...   # all ok

Checks that the new tests catch drift:

Deleting the /v2/sandboxes/{id}/logs entry fails TestPathRoutesCoverOpenAPI with GET /v2/sandboxes/{sandboxID}/logs: path routed = false, want true; update pathRoutes.
Running the shared vectors against the old cluster_list.go fails four cases: the timestamp format, and the token on an exact-limit last page.
The Rust cursor passes the vectors unchanged.

Skipped checks and reasons:

make test-unit capability tests (--ignored) and integration suites: no capability-dependent or runtime code changed. The Rust change is one test.
Scheduler Redis tests: redis-server is not installed on this host. The scheduler packages still pass go test.

Risks and reviewer notes

routes.go is the file to review. Order matters: the first match wins, so PUT .../builder (allocation) and GET .../status|logs (read) come before the any-method build entries.
TestPathRoutesCoverOpenAPI reads the spec by relative path, so the gateway tests must run from a repository checkout, as CI does.

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/375 **Author:** [@yingdi-shan](https://github.com/yingdi-shan) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `refactor/gateway-route-table` --- ### 📝 Commits (2) - [`ba27af9`](https://github.com/kvcache-ai/AgentENV/commit/ba27af903f5af8d98b1fca5ebcfe17f46fe8e156) refactor(gateway): route sandbox and build paths from one table - [`b7723e5`](https://github.com/kvcache-ai/AgentENV/commit/b7723e5dc70c28d082afd265cee19df9afeafe7b) fix(gateway): match node API sandbox list cursors via shared vectors ### 📊 Changes **10 files changed** (+645 additions, -194 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/services-ci.yml` (+4 -0) 📝 `services/gateway/internal/cluster_list.go` (+14 -14) 📝 `services/gateway/internal/cluster_list_test.go` (+105 -3) ➕ `services/gateway/internal/routes.go` (+111 -0) ➕ `services/gateway/internal/routes_test.go` (+108 -0) 📝 `services/gateway/internal/server.go` (+21 -112) 📝 `services/gateway/internal/server_test.go` (+3 -64) 📝 `services/go.mod` (+1 -1) 📝 `src/api/impls/pagination.rs` (+108 -0) ➕ `src/api/testdata/pagination.json` (+170 -0) </details> ### 📄 Description ## What Two commits, both in the gateway: 1. **`refactor(gateway): route sandbox and build paths from one table`.** The gateway's hand-written path parsers (`isSandboxControlPlaneRequest`, `sandboxIDFromPath`, `templateBuildIDFromPath`, `v2TemplateBuildIDFromPath`, `isTemplateBuilderAllocation`, `isForkPath`) become one table, `pathRoutes` in `services/gateway/internal/routes.go`. Each entry names a method, a path pattern, and how the owning node is picked. A new test reads `src/api/openapi.yml` and fails if an operation addressed by `{sandboxID}` or `{buildID}` is missing from the table, or if a table entry matches no operation. 2. **`fix(gateway): match node API sandbox list cursors via shared vectors`.** `src/api/testdata/pagination.json` holds cursor tokens to accept, reject and encode, plus pages to cut from a fixed set of sandboxes. The Rust cursor tests (`src/api/impls/pagination.rs`) and the gateway's cluster-list tests both read it. The gateway is fixed to match the node API. ## Why These came out of a review of why recent feature PRs touch 20–40 files. - **Routing:** every sandbox- or build-scoped endpoint needed edits to several Go path parsers. #338 (logs) and #347 (image builds) both did this. If an edit was missed, nothing failed: the request was routed to a scheduled node instead of the sandbox's node. - **Pagination:** the gateway re-implements the node API's v2 sandbox list cursor in Go (`cluster_list.go`), and nothing kept the two in step. #209 and #263 both had to change both sides. The new vectors show they had already drifted, as listed below. ## Related issue N/A. Follow-up to the review behind #367. ## Scope and non-goals - Routing is unchanged for every request that reaches path routing. Requests the gateway answers itself (lists, node detail, sandbox metrics list) are handled before path routing, as before. - The route table is hand-written Go and checked against the spec, not generated from it. Generating it would need an `x-` extension in `openapi.yml`; that is a possible follow-up. - Not included: image-build routes from #347. When #347 lands, they become table entries (`ownerNewBuildConfirmed` already covers allocations whose ID comes back in `x-agentenv-build-id`). ## Design and behavior changes Route owner kinds, which replace the separate parsers: | Kind | Picks the node by | Records an assignment | |---|---|---| | `ownerSandbox` | sandbox in `{id}` | no | | `ownerSandboxFork` | source sandbox in `{id}` | children from the response | | `ownerBuild` | build in `{id}` | no | | `ownerBuildRead` | build in `{id}`; a build with no binding falls back to scheduling | no | | `ownerNewBuild` | scheduling | `{id}` | | `ownerNewBuildConfirmed` | scheduling | `x-agentenv-build-id`, which must equal `{id}` | Gateway behavior changes from the shared vectors: - **No extra empty page.** The gateway issued `x-next-token` whenever a page was exactly `limit` long. Clients then fetched an empty final page. Nodes issue a token only when more items remain, and the gateway now does the same. - **Token format.** The gateway wrote `RFC3339Nano` timestamps, which drop trailing zeros. Nodes always write nine fraction digits. The gateway now writes the same format. Tokens in the old format are still accepted. ## Compatibility and operations - Public API or generated protocol: no schema change. Gateway list responses no longer include `x-next-token` on a final page that is exactly `limit` long. - Configuration or defaults: none. - Snapshot manifest, artifact layout, or storage format: none. - Upgrade and rollback: next tokens issued by the old gateway still parse. - Host requirements, permissions, ports, or dependencies: `gopkg.in/yaml.v3` moves from indirect to direct in `services/go.mod` (test only; already in `go.sum`). Services CI now also runs when `src/api/openapi.yml` or `src/api/testdata/pagination.json` changes, since the gateway tests read both. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [ ] Relevant Rust integration tests - [x] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text make fmt # clean make clippy # clean cargo test -p agentenv -p envd -p linux-cap --lib # agentenv: 963 passed, 4 ignored; envd: 3 passed make -C services fmt-check vet # clean make -C services build # ok go test ./gateway/... ./scheduler/... ./shared/... ./api/... # all ok ``` Checks that the new tests catch drift: - Deleting the `/v2/sandboxes/{id}/logs` entry fails `TestPathRoutesCoverOpenAPI` with `GET /v2/sandboxes/{sandboxID}/logs: path routed = false, want true; update pathRoutes`. - Running the shared vectors against the old `cluster_list.go` fails four cases: the timestamp format, and the token on an exact-limit last page. - The Rust cursor passes the vectors unchanged. Skipped checks and reasons: - `make test-unit` capability tests (`--ignored`) and integration suites: no capability-dependent or runtime code changed. The Rust change is one test. - Scheduler Redis tests: `redis-server` is not installed on this host. The scheduler packages still pass `go test`. ## Risks and reviewer notes - `routes.go` is the file to review. Order matters: the first match wins, so `PUT .../builder` (allocation) and `GET .../status|logs` (read) come before the any-method build entries. - `TestPathRoutesCoverOpenAPI` reads the spec by relative path, so the gateway tests must run from a repository checkout, as CI does. ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:22:52 +00:00