Repository navigation
fix(sandboxes): align CLI with hosting-2 lifetime contract - #294
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d01df56e5
ℹ️ 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".
| cloud project. Omitted fields preserve current settings. Cloud TTL must be | ||
| 30–28800 seconds. Local mode accepts `0` for unlimited lifetime or a positive TTL. |
There was a problem hiding this comment.
Honor unlimited template lifetimes in local sessions
When a local template is configured with the newly supported ttl_seconds: 0, volcano sandboxes run --template ... still initializes --duration to 3600 and always sends it as MaxDurationSeconds (internal/cmd/sandboxes/exec.go:155-177), overriding the template default; explicitly passing --duration 0 is also rejected. Thus the advertised unlimited lifetime cannot be used by the CLI's session-creation command. Leave the duration omitted unless the flag was supplied, and validate zero according to local/cloud mode.
AGENTS.md reference: AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
|
Post-merge review of this change (approve, with notes). It fixes the local-mode
I did not review the generated client or OpenAPI refresh line by line, and did not run the CLI's CI. |
Fix Sandbox configuration deploys returning HTTP 400 after Hosting #1743 removed
idle_timeout_seconds. Remove the retired manifest field and round-trip expectations, reject it locally, and documentttl_secondsincluding local unlimited lifetime. Refresh the vendored OpenAPI contract and generated client from Hosting729519c2d, including nullable Sandbox session fields.sandboxes runnow omitsmax_duration_secondsunless--durationis supplied, so sessions inherit template lifetimes. Explicit durations accept zero (unlimited) or positive values locally and 30–28800 seconds in cloud mode. This fixes the P2 where an unconditional 3600-second duration overrode unlimited templates.Validation: projectconfig, config commands, generated client, API, Sandbox commands, manifest extraction, and documentation tests pass. Scoped lint, formatting, and generated-code consistency pass. The lifetime regression test reproduces the prior failure and passes after the fix across local/cloud modes, preset/template selectors, and duration boundaries. Local-mode E2E against Hosting
729519c2d(kong/volcano:hosting2-cli-check) passes, including config dry-run/deploy/pull, inherited 600-second expiry, explicit unlimited override, and inherited unlimited lifetime. Remote CI is rerunning for the latest commit.