Repository navigation
feat(sandboxes): add Ruby session and execution facade - #316
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7b8b79eea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a78b94c26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
swkeever
left a comment
swkeever
left a comment
There was a problem hiding this comment.
Approving. Credential routing, the refresh wrapper, block-exception preservation, short-call timeouts, payload snapshots, and frozen IDs from the earlier Codex threads are all addressed.
Inline comments are non-blocking. The positional options Hash on create/exec is the one I'd most like changed before release: it's the only facade not using keywords, and switching later breaks callers.
Separately, a pre-existing issue that this PR doesn't introduce: GeneratedTransportSupport sets configuration.timeout to milliseconds (timeout_milliseconds), but Typhoeus/Ethon treat :timeout as seconds (CURLOPT_TIMEOUT). Configured timeouts are effectively ~1000x too long across the SDK. Worth its own issue.
There was a problem hiding this comment.
Blocking: the existing mutable-state finding remains at lib/volcano/sandbox_response.rb:35 and lib/volcano/sandbox_session.rb:49: session.state.replace("terminated") makes use skip termination without a server response. I reproduced this against the current head; store an owned frozen state and add a regression that verifies observation cannot change cleanup behavior.
4 participants
Summary
Add the Ruby Sandbox facade for one-shot commands, sessions, lifecycle operations, binary files, scoped HTTP access, and backend-managed user grants. Preserve request IDs and command results, refresh granted-user sessions once on HTTP 401, and preserve block exceptions during cleanup.
Use explicit keyword arguments, validate timestamp inputs, expose expiry timestamps as UTC Time values, redact credential-bearing inspect output, and retain service credentials on handles created by a backend. Cleanup accepts terminating/terminated sessions and HTTP 404. Public presets need no credentials; the generated contract accepts catalog-defined preset IDs and uses canonical region names.
HTTP timeouts preserve configured short-call settings and extend command-completion calls only. Auth retries snapshot nested payloads and preserve immutable session identity. Lifecycle state also owns a frozen snapshot, so caller or response mutations cannot bypass block cleanup. Documentation distinguishes one-shot HTTP 504/409 errors from session-command timeout results.
Validation
cb99f31): documentation-only correction uses/workspace/input.binconsistently for the public binary-file example; diff checks passed and remote checks are running.a20be52): all eight remote checks passed, including Quality Gate and full Ruby 3.2/3.4 quality checks. Two caller/response mutation regressions failed before the fix; all 52 local Sandbox regressions, native changed-file lint, and strict Sandbox Steep passed afterward.Coordination
Hosting Kong/volcano-hosting#1354; JavaScript Kong/volcano-sdk-js#270; Python Kong/volcano-sdk-python#372. Hosting
0c31966b7provides the expired-session HTTP 401 fix with passing native HTTP regressions, and owns canonical shared acceptance scenarios. Hosting staging acceptance uses SDK main, so merge the SDK implementations before final cross-language acceptance. No gem publication.