Skip to content

test(runtime): cover MicroSandbox resource parity - #1454

Merged
skevetter merged 4 commits into
mainfrom
codex/microsandbox-resource-parity
Oct 10, 2026
Merged

skevetter merged 4 commits into
mainfrom
codex/microsandbox-resource-parity

Conversation

@skevetter

@skevetter skevetter commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

MicroSandbox driver parity lacks CPU/memory and boot-capacity coverage. Add six shared scenarios for each driver (built-in and published external v0.1.6): provider defaults, explicit overrides of host requirements, zero-value fallback to host requirements, growth to CPU/memory ceilings, and rejection of ceilings below the initial CPU or memory allocation.

The tests inspect active runtime configuration and guest online CPUs/MemTotal, with a non-overlapping 128 MiB kernel reservation allowance. The hotplug scenario provisions 1 CPU/1024 MiB with ceilings of 2 CPUs/1536 MiB through Devsy, uses msb modify on that test-owned VM, and requires guest convergence within 60 seconds. Independent requests above either ceiling must return a restart-required plan containing the requested resource and its derived ceiling change, and preserve active resources, creation time, guest boot ID, and a guest file. Combined growth requires both CPU and memory changes marked live. This validates the boot capacity supplied by each driver; Devsy gains no live-resize API.

A dedicated KVM CI entry installs checksum-pinned MicroSandbox v0.7.7. It runs all twelve resource specs, including eight VM boots and four negative creation cases. Each scenario has a ten-minute deadline; the suite/job deadlines are 20/25 minutes. Guest convergence bounds follow the pinned runtime's live-resize tests. Existing lifecycle, image and mount suites remain required.

Validation:

  • Local uncached race tests and vet for e2e/tests/up, e2e/framework, and pkg/driver/microsandbox passed. Ginkgo dry run selected all twelve new specs; dry run is not VM evidence.
  • Strict CLI lint (zero issues), all thirteen repository hooks, formatting/actionlint and module verification passed. Final fresh committed local CodeRabbit reviewed all three changed files with zero findings. Published commits have valid GitHub signatures and Samuel K identity.
  • Guest convergence retries SSH errors within the original 60-second bound, matching the pinned upstream live-resize tests; malformed resource output still fails immediately.
  • Assertion regression checks reject wrong or missing resource fields and accept the runtime's derived CPU/memory ceiling plans. Pinned upstream desired_resources raises desired ceilings to the requested target, and the planner classifies those ceiling changes as restart-required. Effective-resource, creation-time, boot-ID and file preservation checks remain unchanged.
  • All 75 final-head implementation CI jobs passed, including the two lifecycle, six image, ten mount and twelve resource specs against both drivers. Fresh Greptile completed at 5/5 with no actionable issues. Full remote CodeRabbit reviewed all three files at the final head with no actionable findings or retained architecture concerns; its generic docstring-coverage warning was independently dispositioned for private E2E helpers. Local MicroSandbox v0.7.2 is below the supported parity minimum; native parity evidence must come from supported-runtime CI.

The built-in provider remains available. Storage capacity, ephemeral roots, egress, prebuild/dockerless behavior and other remaining D5 cases are outside this PR.

Summary by CodeRabbit

  • Tests

    • Added end-to-end checks for resource sizing and hotplug behavior across the built-in and external MicroSandbox providers, including configured limits and rejected changes.
    • Added Linux CI coverage for the resource-parity test suite.
  • Documentation

    • Updated provider runtime protocol guidance with resource-sizing scenarios, validation details, and cutover considerations.

@netlify

netlify Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 60d1abf
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6ac9f8151134a400080c1810

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 839866a9-ae82-482c-8bb5-a6a6baa29417



📥 Commits

Reviewing files that changed from the base of the PR and between 74810dd and 60d1abf.




📒 Files selected for processing (3)
  • .github/workflows/pr-ci.yml
  • e2e/tests/up/provider_microsandbox_resources.go
  • sites/docs-devsy-sh/content/docs/developing-providers/runtime-protocol.mdx



Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.





📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request adds end-to-end resource parity tests for builtin and external MicroSandbox providers. It checks resource sizing, guest capacity, startup rejection, and hotplug behavior. The CI workflow runs the suite, and the provider protocol documentation describes its coverage.

Changes

MicroSandbox resource parity

Layer / File(s) Summary
Resource sizing and parity cases
e2e/tests/up/provider_microsandbox_resources.go
Tests check default resources, provider-option overrides, zero-valued options, and active VM and guest resources. Guest memory checks allow up to 128 MiB below the requested amount.
Resource ceilings and integration coverage
e2e/tests/up/provider_microsandbox_resources.go, .github/workflows/pr-ci.yml, sites/docs-devsy-sh/content/docs/developing-providers/runtime-protocol.mdx
Tests check startup rejection below effective resource ceilings and hotplug changes within and beyond configured ceilings. Rejected hotplug requests must preserve active resources, VM creation time, boot ID, and a guest marker. CI runs the suite, and the documentation describes its scenarios, deadlines, and parity warning.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other





Merge Risk: ⚪ Minimal · up to 60d1a

The resource suite’s over-ceiling checks match the supported runtime behavior. No identified issue needs resolution before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 60d1a

The change adds resource tests and another CI job using existing VM execution privileges. It does not introduce a production resize API or enable application-secret access. Remaining uncertainty concerns upstream runtime behavior during interruption, repetition, and concurrent modification.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The added execution surface is the CI runner, its test VMs, and inherited job credentials and cache access. PR-controlled test code already executes through the same privileged integration path at the base revision. No new production tenant boundary or greater credential authority is established by this change.

Trust Boundaries and Controls

  • observed — The suite requires runtime availability in CI rather than silently skipping missing prerequisites. It uses workspace-resolved VM names and fixed resource requests, preserving the established test-to-runtime boundary instead of accepting a new externally supplied resize target.

Resilience and Maintainability Implications

  • observed — The successful combined resize must report both changes as live and converge in the guest. Sequential over-ceiling requests must remain unapplied, preserve active resources after each request, and preserve creation time, boot ID, and guest data afterward. Deferred workspace cleanup uses a fresh bounded context detached from spec cancellation. These are test assertions and cleanup controls, not proof of upstream atomicity or recovery under concurrent requests or forced interruption.



Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding runtime tests for MicroSandbox resource parity.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (2 skipped: 2 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR





  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 60d1abf
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6ac9f81591ff960008b77914

@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with no new actionable issues found.

Summary

Adds CPU, memory, and boot-ceiling tests for both MicroSandbox providers.

  • MicroSandbox resource checks now cover both provider drivers.

Reviews (2) · Last reviewed commit: "test(runtime): expect derived capacity p..." · Reviewed by Greptile

Comment thread e2e/tests/up/provider_microsandbox_resources.go Outdated
@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@skevetter

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 21 minutes.

@skevetter

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@skevetter

Copy link
Copy Markdown
Contributor Author

Final full CodeRabbit run 839866a9-ae82-482c-8bb5-a6a6baa29417 covers all three PR files at 60d1abf. I inspected the entire review, including Security Architecture and Merge Risk: no actionable findings or retained architecture concerns. The docstring-coverage warning is a no-change disposition: these six unexported E2E helpers have descriptive names and retain rationale comments for convergence, memory allowance and subprocess boundaries. Adding restatements solely to meet the generic coverage percentage would not clarify their contract; repository strict lint and all hooks pass. Existing actionable review threads are resolved.

@skevetter
skevetter marked this pull request as ready for review October 10, 2026 10:05
@skevetter
skevetter merged commit 1a9efbb into main Oct 10, 2026
97 checks passed
@skevetter
skevetter deleted the codex/microsandbox-resource-parity branch October 10, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant