refactor: split bounded query runners - #6762
Conversation
Introduce trusted Docker and gVisor QueryRunner backends with centralized launch derivation and label-scoped lifecycle reconciliation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7cf71cc5-3209-47c7-8743-f903cbe4eff1
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
🟡 Not ready to approve
Shutdown can race with request admission and leave a query container unreconciled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Refactors bounded-query execution into trusted Docker and gVisor runner backends with centralized sandbox specification and lifecycle cleanup.
Changes:
- Adds backend-specific query runners and immutable launch specifications.
- Adds label-based cleanup and shutdown reconciliation.
- Propagates normalized backend configuration and expands tests.
File summaries
| File | Description |
|---|---|
tests/integration/bounded-query-isolation.test.ts |
Uses the Docker backend configuration. |
src/services/bounded-query-service.ts |
Passes the normalized backend to the broker. |
src/services/bounded-query-service.test.ts |
Tests backend environment mapping. |
src/bounded-query/query-runner.test.ts |
Covers runner selection, specs, and cleanup. |
src/bounded-query/end-to-end.test.ts |
Updates end-to-end configuration. |
src/bounded-query/broker.test.ts |
Updates broker and launch-argument tests. |
containers/bounded-query/Dockerfile |
Validates new broker modules. |
containers/bounded-query/broker/server.js |
Creates runners and reconciles shutdown state. |
containers/bounded-query/broker/query-runner.js |
Defines and selects runner backends. |
containers/bounded-query/broker/query-runner-spec.js |
Derives trusted container specifications. |
containers/bounded-query/broker/gvisor-query-runner.js |
Implements fixed-runsc execution. |
containers/bounded-query/broker/docker-query-runner.js |
Implements Docker execution and cleanup. |
containers/bounded-query/broker/docker-client.js |
Encapsulates bounded Docker CLI calls. |
containers/bounded-query/broker/config.js |
Loads and validates backend selection. |
containers/bounded-query/broker/broker.js |
Requires an injected runner and exposes draining. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| await Promise.race([ | ||
| broker.drain(), | ||
| new Promise((resolve) => setTimeout(resolve, SHUTDOWN_GRACE_MS)), | ||
| ]); | ||
| await runner.reconcileRun(runId); |
|
@copilot address review feedback |
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
❌ Smoke Claude failed |
|
❌ Smoke Copilot BYOK reports failed. BYOK mode investigation needed... |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
❌ Smoke Gemini reports failed. Facets need polishing... |
|
❌ Security Guard failed. Please review the logs for details. |
|
📰 DEVELOPING STORY: Smoke Docker Sbx reports failed. Our correspondents are investigating the incident... |
|
Chroot tests failed Smoke Chroot failed - See logs for details. |
|
🔌 Smoke Services — Service connectivity failed |
|
🛡️ Smoke Copilot Network Isolation reports failed while checking network isolation. Investigate the egress model. |
|
❌ Contribution Check failed. Please review the logs for details. |
|
📰 DEVELOPING STORY: Smoke Copilot reports failed. Our correspondents are investigating the incident... |
|
Build Test Failed Build Test Suite - See logs for details |
|
📡 Smoke OTel Tracing reports failed. OTel tracing regression detected. |
|
✅ Contribution Check completed successfully! PR #6762 follows the contribution guidelines in CONTRIBUTING.md: the description is clear and references prior work (#6758 / 10885c9), new functionality is covered by focused unit and integration tests, and new files are placed under the expected src/ and containers/ directories. No review comment needed. |
|
❌ Security Guard failed. Please review the logs for details. |
|
📰 DEVELOPING STORY: Smoke Docker Sbx reports failed. Our correspondents are investigating the incident... |
|
📡 Smoke OTel Tracing reports failed. OTel tracing regression detected. |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
Copilot Network Isolation Smoke Test (@lpcox) EGRESS_RESULT allow=pass deny=pass
Overall status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
Smoke Test: Claude Engine ValidationOverall result: PASS ✅
Event:
|
|
Smoke Test: Services Connectivity
Overall: FAIL — host.docker.internal could not be resolved from within the AWF sandbox.
|
|
@lpcox
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) Overall: PASS
|
|
Smoke Test Results (@lpcox)
Recent merged PRs:
Overall: PASS
|
|
Merged PRs:
Checks:
Overall status: FAIL
|
Smoke Test: Copilot BYOK (Direct Mode) ✅ PASSTest Results:
Mode: Running via cc/ @lpcox
|
Chroot Version Comparison
Overall: FAILED — Node.js version mismatch between host and chroot environments.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Note: Java tests required overriding Maven's local repository path (
|
Smoke Test: Gemini Engine Validation
Overall status: FAIL
|
Summary
Layer 2 of the bounded-query multi-sandbox work, built directly on layer 1 (#6758 /
10885c9d).QueryRunnercontract with explicitDockerQueryRunnerandGvisorQueryRunnerbackendsdockerversusgvisorfrom normalized AWF configuration; only the gVisor backend emits--runtime runsc, and missing/unknown runtimes fail closed without fallbackValidation
/var/tmpversus/private/var/tmppath-assumption tests fail outside this diff