feat(ui): add CPU and memory defaults to sandbox create command - #245
Conversation
Sandbox example commands now include --cpu 0.2 --memory 512Mi so users get resource limits out of the box, sized for lightweight POC workloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Replace --cpu/--memory with --driver-config-json to set separate requests (100m CPU, 512Mi memory) and limits (500m CPU, 512Mi memory), lowering the scheduling footprint while allowing burst headroom. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the --driver-config-json value into a DRIVER_CONFIG variable above the sandbox create command so the JSON blob does not clutter the command itself. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
The change is small, correct, and well-tested: buildSandboxCreateCommand now emits a DRIVER_CONFIG shell variable plus --driver-config-json "$DRIVER_CONFIG", the deterministic JSON.stringify output matches the updated assertions, and the spec is updated to require the new defaults. My main concerns are consistency (a second surface presents the same recommended command and was not updated) and contract verification of the new flag/schema.
Summary
This PR enriches the copyable "Create a sandbox" command in the gateway Connection tab with CPU/memory requests and limits via a --driver-config-json payload, and updates the web-console architecture spec to mandate those defaults. The implementation is clean and the modified test assertions faithfully mirror the new output (no removed guarantees, no optional→required tightening on existing data).
Findings
[Major] The CLI-rendered recommended command was not updated, so two surfaces now diverge
components/cli/cmd/hypershell/get/gateway/cmd.go:166-171 builds the same recommended openshell sandbox create walkthrough that buildSandboxCreateCommand builds for the web console. This PR adds the resource requests/limits to the UI copy only; the CLI (openshell get gateway) still emits a create command with no --driver-config-json. Users copying the command from the CLI get no resource defaults, which undercuts the stated goal of establishing lightweight POC defaults on shared clusters. Either update the CLI helper to match, or record explicitly why the two surfaces intentionally differ. Confidence: High that they diverge; Medium on whether the CLI is in scope for this PR.
[Minor] Verify the --driver-config-json flag and nested schema contract before shipping it as the recommended command
The recommended command now hard-codes --driver-config-json with the kubernetes.containers.agent.resources.{requests,limits} shape. Per the project convention "verify contracts and references," please confirm the installed openshell CLI accepts this flag and this exact JSON schema; if the flag name or nesting is wrong, the primary copy-paste command in the Connection tab fails at runtime. Confidence: Medium.
[Minor] buildSandboxCreateCommand lacks a doc comment explaining the DRIVER_CONFIG preamble
Sibling exported builders (buildProviderCreateCommand, buildInferenceSetCommand, buildSetupScript) carry comments explaining their shape. A short note on why the command emits a shell-variable preamble (readability / avoiding an unwieldy inline JSON argument) would help future maintainers. Confidence: High.
Cross-PR coordination
No material cross-PR coordination issue requires maintainer action.
Findings Summary (ordered by severity, highest first)
- [Major] CLI
openshell get gatewayrecommended sandbox-create command not updated to match the new resource defaults - Cross-surface consistency (cmd.go:166-171) - [Minor] Verify
--driver-config-jsonflag and nested resource schema contract - Contract verification (gateway-connections.ts L136-L141) - [Minor] Missing doc comment on
buildSandboxCreateCommand- Docs (gateway-connections.ts L119)
Convention Checklist
| Convention | Result |
|---|---|
| Test Diff Scrutiny (modified assertions justified, no hidden contract change) | Pass |
Input validated / no injection (JSON built via JSON.stringify, no user interpolation) |
Pass |
| No secrets in logs or responses | Pass (N/A values) |
| Config separate from code | Pass |
| Image/command references consistent across the stack | Fail |
| Verify contracts and references | Needs verification |
| Conventional commit messages | Pass |
| const command = [ | ||
| "openshell sandbox create", | ||
| `--name ${name}`, | ||
| '--driver-config-json "$DRIVER_CONFIG"', |
There was a problem hiding this comment.
[Minor] Verify the --driver-config-json contract. This makes --driver-config-json and the kubernetes.containers.agent.resources.{requests,limits} schema the primary, copy-paste command shown in the Connection tab. Per the "verify contracts and references" convention, please confirm the installed openshell CLI accepts this exact flag name and JSON nesting; if either is wrong, the recommended command fails at runtime for every user. Confidence: Medium.
| model: string = claudeModel, | ||
| ): string { | ||
| return [ | ||
| const driverConfig = JSON.stringify({ |
There was a problem hiding this comment.
[Minor] Add a doc comment. Sibling exported builders (buildProviderCreateCommand, buildInferenceSetCommand, buildSetupScript) explain their shape. A one-line comment on why this emits a DRIVER_CONFIG shell-variable preamble (readability vs. an unwieldy inline JSON argument) would aid future maintainers. Confidence: High.
| 2. **Add a Claude on Vertex AI provider.** The step SHALL present, as its primary read-only Clipboard Copy value, one `openshell provider create` command for the `google-vertex-ai` provider that reads credentials and configuration from the user's own environment rather than requiring hand-edited secrets. An expandable details disclosure SHALL document the Application Default Credentials prerequisite, the environment variables the command reads, the command that routes a Claude model through the provider, and the sandbox routing caveats, including reaching Vertex through `inference.local` and not setting `CLAUDE_CODE_USE_VERTEX` inside the sandbox. The step SHALL NOT display, request, or persist Google Cloud credentials, service-account keys, project identifiers, or access tokens in the browser. | ||
|
|
||
| 3. **Create a sandbox.** The step SHALL present an `openshell sandbox create` command that names a sandbox, attaches the provider from step 2, and launches the agent, as a read-only Clipboard Copy value. | ||
| 3. **Create a sandbox.** The step SHALL present an `openshell sandbox create` command that names a sandbox, specifies CPU and memory resource requests and limits via a `--driver-config-json` flag, attaches the provider from step 2, and launches the agent, as a read-only Clipboard Copy value. The default resource values SHALL be CPU requests `100m`, CPU limits `500m`, memory requests `512Mi`, and memory limits `512Mi`. |
There was a problem hiding this comment.
[Major] Keep the two recommended-command surfaces in sync. This spec now mandates the --driver-config-json resource defaults for the web console's create-sandbox step, but the CLI helper that renders the same recommended walkthrough (components/cli/cmd/hypershell/get/gateway/cmd.go:166-171) still emits a create command with no resource config. Update the CLI helper to match, or state explicitly why the surfaces intentionally differ. Confidence: High that they diverge.
bsquizz
left a comment
bsquizz
left a comment
There was a problem hiding this comment.
Amber Analysis
This PR adds CPU and memory resource limits to sandbox creation commands. The changes are spec-compliant and tested, but introduce a subtle breaking change to the buildSandboxCreateCommand function signature that should be acknowledged. Overall: APPROVE with minor suggestions.
Findings Summary
- [Minor] Shell quoting could be more robust - Code Quality (gateway-connections.ts:136)
- [Minor] Hard-coded resource values with no override mechanism - API Design (gateway-connections.ts:123-134)
- [Minor] Undocumented breaking change to function contract - API Design (gateway-connections.ts:120-147)
Convention Checklist
| Convention | Result |
|---|---|
| Spec consistency | Pass |
| Test coverage | Pass |
| No secrets in code | Pass |
| Conventional commit messages | Pass |
Detailed Findings
1. [Minor] Shell Quoting Robustness
File: packages/gateway-management-ui/src/gateways/gateway-connections.ts:136
const variable = `DRIVER_CONFIG='${driverConfig}'`;Issue: Single quotes around JSON work here but are brittle. If driverConfig ever contains a single quote (escaped property names, localized content, etc.), this breaks shell parsing.
Recommendation: Either document the quoting strategy or use a more robust approach:
// Option 1: Document the assumption
const variable = `DRIVER_CONFIG='${driverConfig}'`; // JSON never contains single quotes
// Option 2: Use proper shell escaping
const variable = `DRIVER_CONFIG=${JSON.stringify(driverConfig)}`;Confidence: High (85%)
2. [Minor] Hard-coded Resource Values
File: packages/gateway-management-ui/src/gateways/gateway-connections.ts:123-134
Issue: CPU and memory values are hard-coded with no way to override. Future requirements for different resource profiles (dev vs. prod, light vs. heavy workloads) will require code changes.
Recommendation: Accept resource limits as optional parameters with the current values as defaults:
export function buildSandboxCreateCommand(
name: string = sandboxName,
model: string = claudeModel,
resources?: { cpu?: string; memory?: string },
): string {
const { cpu = "500m", memory = "512Mi" } = resources ?? {};
// ...
}Confidence: Medium (75%) - depends on whether resource customization is a future requirement
3. [Minor] Undocumented Breaking Change
File: packages/gateway-management-ui/src/gateways/gateway-connections.ts:120-147
Issue: The function previously returned a single multi-line command string. It now returns a shell script (variable declaration + newlines + command). This changes the contract for any consumer parsing or processing the output.
Current callers are safe (they copy the whole thing to clipboard), but the change should be acknowledged.
Recommendation: Add a JSDoc comment documenting the new return format:
/**
* Builds a complete shell script for creating an OpenShell sandbox with resource limits.
* Returns a multi-line string containing:
* 1. DRIVER_CONFIG variable assignment with JSON resource specification
* 2. openshell sandbox create command referencing $DRIVER_CONFIG
*/
export function buildSandboxCreateCommand(Confidence: High (90%)
Positive Observations
✅ Spec alignment: specs/web-console/architecture.spec.md correctly updated with default resource values
✅ Test coverage: Both unit tests updated to match new command format
✅ Readability: Extracting driver config into a variable improves readability
✅ Conventional commits: Commit messages follow project standards
Overall Assessment: APPROVE
The PR successfully implements resource limits as specified. All findings are Minor severity - nice-to-haves that improve robustness but don't block merge.
Rollback instructions: Revert to commit ea34c16 if resource limits cause sandbox creation failures.
bsquizz
added
amber/approved
Address PR feedback by adding CPU/memory resource limits to the CLI-rendered sandbox create command, matching the UI's --driver-config-json output. Add JSDoc comment documenting buildSandboxCreateCommand's return format. Fixes cross-surface consistency issue where CLI and UI diverged on recommended sandbox resource configuration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
This PR cleanly adds CPU/memory resource requests and limits to the suggested openshell sandbox create command across the CLI script, the shared UI command builder, and the web-console spec. The change is small, the CLI and UI produce identical JSON, and tests are updated; I found no security or correctness blockers, only minor drift/coordination concerns.
What's good
- The
DRIVER_CONFIGJSON emitted by the CLI (cmd.go) and the object serialized bybuildSandboxCreateCommand(gateway-connections.ts) match byte-for-byte, and both agree with the values now pinned inarchitecture.spec.mdstep 3. - Wrapping the JSON in a single-quoted
DRIVER_CONFIGshell variable keeps the copyable command readable and avoids quoting pitfalls inline. - The
EditableCommandrenderstemplateCommandinside aCodeBlock, so the new multi-line variable-plus-command block displays and copies correctly; thesandbox/modelmarkers are still present in the command portion.
Findings
[Minor] Resource defaults are duplicated in three places (drift risk). The exact request/limit values now live independently in components/cli/cmd/hypershell/get/gateway/cmd.go, packages/gateway-management-ui/src/gateways/gateway-connections.ts, and specs/web-console/architecture.spec.md. A future change to any single value must be mirrored by hand in the other two. The pinned spec values and the UI unit test partially guard against drift, but the CLI string has no test asserting its content, so a CLI-only edit could silently diverge from the UI. Consider a shared constant or at least a note tying these together. Confidence: High.
[Minor] Unverified assumption about the driver-config schema. The config hardcodes the container key agent and the kubernetes.containers.<name>.resources shape. If the OpenShell sandbox driver names the primary container differently (or nests resources elsewhere), the suggested command would be silently ignored or rejected. Please confirm this matches the OpenShell --driver-config-json contract for the sandbox driver in use. Confidence: Low.
[Minor] Test assertion changed in place. The two buildSandboxCreateCommand() expectations in gateway-connections.test.ts were rewritten to expect the new DRIVER_CONFIG prefix and --driver-config-json flag. This is a legitimate, intended contract change (not a removed guarantee) and is called out in the PR description, so no action is required beyond noting it. Confidence: High.
Cross-PR coordination
Another open pull request also rewrites buildSandboxCreateCommand in packages/gateway-management-ui/src/gateways/gateway-connections.ts and edits the same Connection-tab section of specs/web-console/architecture.spec.md, in a way that conflicts by design rather than just by text:
- #208 changes the create command's
--name ${name}to--name ${shellArgument(name)}(adding shell-argument escaping of the operator-editable sandbox name), restructures the spec's Connection tab from three to four ordered steps (adding a "Connect to a sandbox" step), and updates the samebuildSandboxCreateCommandexpectations ingateway-connections.test.ts. This PR instead fully rewritesbuildSandboxCreateCommandto prepend aDRIVER_CONFIGvariable and a--driver-config-jsonflag while leaving--name ${name}unescaped, and rewrites the same step-3 spec text and the same test assertions. These are competing rewrites of one function and one spec section: whichever merges second must re-apply the other's intent, and a naive merge would drop #208's shell-argument escaping of the sandbox name or lose this PR's resource defaults. Maintainers should decide a merge order and confirm the reconciled builder both escapes the name and injects the driver config, and that the spec reflects four steps with the step-3 resource-limit wording.
Findings Summary (ordered by severity, highest first)
- [Minor] Resource defaults duplicated across CLI, UI, and spec with no CLI-side test - Config/Code Separation (cmd.go L166, gateway-connections.ts L132)
- [Minor] Unverified
agentcontainer name / driver-config JSON schema - Verify Contract (cmd.go L166, gateway-connections.ts L132) - [Minor] In-place test assertion rewrite (justified, intended) - Test Diff Scrutiny (gateway-connections.test.ts)
Convention Checklist
| Convention | Result |
|---|---|
| No secrets in logs or responses | Pass |
| Input validated | Pass |
| Image/config references consistent across the stack | Pass (CLI, UI, and spec JSON match) |
| Config separate from code | Minor (values duplicated in three places) |
| Verify contracts and references | Minor (driver-config schema unconfirmed) |
| Test diff scrutiny (modified assertions justified) | Pass |
| Conventional commit messages | Pass |
| "openshell inference set --provider " + providerName + " --model " + model, | ||
| "", | ||
| "# 4. Create a sandbox", | ||
| `DRIVER_CONFIG='{"kubernetes":{"containers":{"agent":{"resources":{"requests":{"cpu":"100m","memory":"512Mi"},"limits":{"cpu":"500m","memory":"512Mi"}}}}}}'`, |
There was a problem hiding this comment.
The resource request/limit JSON is hardcoded here and independently in the UI builder (gateway-connections.ts) and pinned again in architecture.spec.md. There is no test asserting this CLI script content, so a future edit to any single copy can silently diverge. Consider a shared source of truth or a comment cross-referencing the other two locations. Also confirm the agent container key and the kubernetes.containers..resources shape match the OpenShell --driver-config-json contract, otherwise the flag may be ignored or rejected.
| model: string = claudeModel, | ||
| ): string { | ||
| return [ | ||
| const driverConfig = JSON.stringify({ |
There was a problem hiding this comment.
These defaults duplicate the JSON emitted by the CLI script and the values pinned in the spec; keep all three in sync (a shared constant would remove the drift risk). Note the sandbox name is still interpolated as --name ${name} without shellArgument(...) here; another open PR adds that escaping to this same function, so a merge will need to reconcile both changes.
Extract hardcoded CPU/memory resource values into named constants (sandboxResourceDefaults in TS, sandboxDriverConfig in Go) with cross-reference comments to prevent drift between the CLI, UI, and spec. Apply shellArgument() to the sandbox name in buildSandboxCreateCommand for consistency with other command builders. Add CLI and UI tests asserting the resource config content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
This change adds default CPU/memory resource requests and limits to the suggested openshell sandbox create command across the UI, the CLI-rendered connection script, and the web-console spec, factoring the values into named constants to prevent drift. The implementation is clean, well-tested, and cross-surface consistent; I have no blocking findings, but one cross-PR coordination item needs a maintainer decision before merge.
Summary
Good, focused change. I verified that the three surfaces agree byte-for-byte: JSON.stringify of the TS sandboxResourceDefaults-backed object produces exactly the Go sandboxDriverConfig literal ({"kubernetes":{"containers":{"agent":{"resources":{"requests":{"cpu":"100m","memory":"512Mi"},"limits":{"cpu":"500m","memory":"512Mi"}}}}}}), and both match the spec's stated defaults (CPU requests 100m, limits 500m; memory 512Mi/512Mi). The JSON is safely single-quoted inside DRIVER_CONFIG='...' and referenced as "$DRIVER_CONFIG", so there is no shell-quoting problem in the emitted script.
The modified existing test in gateway-connections.test.ts updates the expected buildSandboxCreateCommand() output to reflect the new two-part script; this is a legitimate expectation update for changed behavior (not a silently removed guarantee), and it is backed by additional assertions plus the shell-metacharacter quoting test. No secrets, no panic(), no reconcile/security-context surface touched.
Findings
- [Minor] Contract verification - The emitted commands depend on
openshell sandbox createaccepting a--driver-config-jsonflag and on thekubernetes.containers.agent.resourcesschema shape. This is an external OpenShell CLI contract; the PR's own test plan leaves "run against a live gateway" unchecked. Per CLAUDE.md ("Verify contracts and references"), confirm the flag and schema against the OpenShell version users are directed to install before merging. Confidence: Medium. - [Minor] Drift risk - The Go
sandboxDriverConfigis a hand-maintained raw JSON literal while the TS side builds the same JSON from the structuredsandboxResourceDefaultsconstant. They match today and the cross-reference comments are a good safeguard, but nothing fails CI if only one side changes. Consider a follow-up guard (or at minimum keep the cross-ref comments authoritative). Confidence: High.
Cross-PR coordination
Another open pull request (#208, "Adding section to gateway UI with instructions for sandbox connecting") edits the same buildSandboxCreateCommand function and the same "Create a sandbox" step in specs/web-console/architecture.spec.md, and it independently applies the identical --name ${name} -> --name ${shellArgument(name)} change that this PR also makes. This is a competing/duplicate change to a shared interface and spec section, not just file overlap: this PR rewrites buildSandboxCreateCommand to return a two-part DRIVER_CONFIG + command script, while #208 adds a new sandbox-connect step whose UI shares the create step's sandbox-name state. Maintainers should decide a merge order and reconcile the two designs so the create step ends up with both the driver-config resource defaults and the adjacent connect step, and so the duplicated shellArgument(name) edit is de-duplicated rather than causing a conflicting or double-applied change.
Findings Summary (ordered by severity, highest first):
- [Minor] External
--driver-config-jsonflag / resource schema is an unverified OpenShell CLI contract - Contract verification (cmd.go L175, gateway-connections.ts L155) - [Minor] Go JSON literal and TS structured constant can drift with no CI guard - Spec/Code consistency (cmd.go L89)
Convention Checklist:
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
| No secrets in logs or responses | Pass |
| Input validated / shell quoting | Pass |
| Image references consistent across surfaces | Pass |
| Test diff scrutiny (no removed guarantee) | Pass |
| Conventional commit messages | Pass |
| Spec updated to match code | Pass |
| // Keep in sync with: | ||
| // packages/gateway-management-ui/src/gateways/gateway-connections.ts (sandboxResourceDefaults) | ||
| // specs/web-console/architecture.spec.md § Create a sandbox | ||
| const sandboxDriverConfig = `{"kubernetes":{"containers":{"agent":{"resources":{"requests":{"cpu":"100m","memory":"512Mi"},"limits":{"cpu":"500m","memory":"512Mi"}}}}}}` |
There was a problem hiding this comment.
This Go literal is maintained by hand while the TS side (sandboxResourceDefaults) builds the same JSON from a structured constant. I verified they match exactly today (JSON.stringify(...) produces this string byte-for-byte) and the cross-reference comments are a good safeguard, but nothing in CI fails if only one side is edited later. Consider a follow-up cross-surface guard. [Minor]
| "", | ||
| "openshell sandbox create \\", | ||
| " --name " + sandboxName + " \\", | ||
| ` --driver-config-json "$DRIVER_CONFIG" \`, |
There was a problem hiding this comment.
The emitted command relies on openshell sandbox create supporting --driver-config-json and on the kubernetes.containers.agent.resources schema shape. This is an external OpenShell CLI contract and the PR's test plan leaves the live-gateway verification unchecked. Per CLAUDE.md ("Verify contracts and references"), please confirm the flag and schema against the OpenShell version users are told to install. [Minor]
| "openshell sandbox create", | ||
| `--name ${name}`, | ||
| `--name ${shellArgument(name)}`, | ||
| '--driver-config-json "$DRIVER_CONFIG"', |
There was a problem hiding this comment.
Same external-contract note as the CLI side: --driver-config-json and the resources JSON shape must be validated against the target OpenShell CLI version. Also note PR #208 independently changes the adjacent --name ${name} line to --name ${shellArgument(name)} (identical to this PR) and edits the same spec step; coordinate the merge to avoid a duplicate/competing change. [Minor]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Amber reviewStatus: Complete |
jsell-rh
left a comment
jsell-rh
left a comment
There was a problem hiding this comment.
Verdict
APPROVE-quality change with two coordination/observation items worth a look before merge. The functional change is clean, low-risk, and well-tested: the create-sandbox command now advertises CPU/memory requests and limits via --driver-config-json, the Go CLI and the React package emit byte-identical JSON, and the spec was updated to match.
I confirmed the two generated configs are identical (JSON.stringify of sandboxResourceDefaults equals the Go sandboxDriverConfig literal, and both match the values now written into the spec). The sandbox name is also now passed through shellArgument, which is a small correctness improvement over the previous unquoted interpolation.
Findings
Minor - resource JSON is duplicated in three places with only comment-based sync. The same {"kubernetes":{"containers":{"agent":{"resources":...}}}} payload now lives as a raw Go string literal (cmd.go), a structured TS constant (gateway-connections.ts), and prose in architecture.spec.md. The "keep in sync" comments are helpful, but nothing fails when they drift. Since three-way drift here would silently ship a CLI command that disagrees with the UI command and the spec, consider a lightweight guard (e.g. a Go test that unmarshals sandboxDriverConfig and asserts the four values, mirroring the TS test that already asserts against sandboxResourceDefaults). Confidence: High.
Test Diff Scrutiny. The modified assertions in gateway-connections.test.ts (the "creates a sandbox" expectations) are additive - they extend the expected command to include the new DRIVER_CONFIG prefix and --driver-config-json line rather than flipping a previously-proven guarantee. No optional-to-required contract was silently removed. This is a legitimate update, not a masked breaking change.
Cross-PR coordination
Another open pull request adds a fourth "Connect to a sandbox" step to the same Connection walkthrough and independently rewrites buildSandboxCreateCommand in packages/gateway-management-ui/src/gateways/gateway-connections.ts (it changes --name ${name} to --name ${shellArgument(name)}), edits the same specs/web-console/architecture.spec.md "Create a sandbox" section, and updates the same gateway-connections.test.ts assertions on the exact create-sandbox command output. This is a competing redefinition of the same function and its contract, not just file overlap: whichever merges second must reconcile the final command shape (this PR turns the single-line command into a multi-line DRIVER_CONFIG=... + command block, while the other PR assumes the single-line form and shares sandbox-name state with its new connect step), and both PRs' hard-coded expected-output tests and spec step text must be unified. Maintainers should pick a merge order and decide on the merged command format so the two efforts do not overwrite each other's create-sandbox contract.
Findings Summary (ordered by severity, highest first):
- [Minor] Resource JSON duplicated across Go literal, TS constant, and spec prose with only comment-based sync; no automated guard against drift - Spec/Code Consistency (cmd.go L90, gateway-connections.ts L89, architecture.spec.md L503)
Convention Checklist:
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
| No secrets in logs or responses | Pass |
| Input validated / shell-arg quoting | Pass |
| Image/config references consistent across stack | Pass |
| Conventional commit messages | Pass |
| Test diff scrutiny (no silently removed guarantees) | Pass |
| // | ||
| // packages/gateway-management-ui/src/gateways/gateway-connections.ts (sandboxResourceDefaults) | ||
| // specs/web-console/architecture.spec.md § Create a sandbox | ||
| const sandboxDriverConfig = `{"kubernetes":{"containers":{"agent":{"resources":{"requests":{"cpu":"100m","memory":"512Mi"},"limits":{"cpu":"500m","memory":"512Mi"}}}}}}` |
There was a problem hiding this comment.
This raw JSON literal is the most drift-prone of the three copies of the resource config (the TS side is built structurally from sandboxResourceDefaults, and the spec states the values in prose). A small Go test that unmarshals sandboxDriverConfig and asserts cpu 100m/500m and memory 512Mi/512Mi would turn the "keep in sync" comment into an enforced contract. Minor.
| // Sandbox resource defaults -- keep in sync with: | ||
| // components/cli/cmd/hypershell/get/gateway/cmd.go (sandboxDriverConfig) | ||
| // specs/web-console/architecture.spec.md § Create a sandbox | ||
| export const sandboxResourceDefaults = { |
There was a problem hiding this comment.
Nice that this constant is the single source for the TS side and that a test already asserts the emitted JSON against it. The remaining gap is cross-language: this and the Go sandboxDriverConfig literal plus the spec values are only kept aligned by comments. Consider documenting which of the three is canonical so a future change updates the others deliberately. Minor.
2 participants
Summary
openshell sandbox createcommand in the UI connection tab--driver-config-jsonwith aDRIVER_CONFIGvariable for readability, setting separate requests (100m CPU, 512Mi memory) and limits (500m CPU, 512Mi memory)--driver-config-jsonSized for lightweight POC workloads on shared clusters -- 100m CPU requests keeps the scheduling footprint low while 500m limits allow burst capacity.
Test plan
DRIVER_CONFIGvariable and--driver-config-jsonflag🤖 Generated with Claude Code