Skip to content

docs(sandbox): clarify OpenShell job isolation - #387

Merged
rapids-bot[bot] merged 5 commits into
NVIDIA-AI-Blueprints:release/2.2from
KyleZheng1284:docs/openshell-job-isolation
Jul 24, 2026
Merged

docs(sandbox): clarify OpenShell job isolation#387
rapids-bot[bot] merged 5 commits into
NVIDIA-AI-Blueprints:release/2.2from
KyleZheng1284:docs/openshell-job-isolation

Conversation

@KyleZheng1284

@KyleZheng1284 KyleZheng1284 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Overview

Document AI-Q's actual OpenShell execution boundary for architecture and security review, and present OpenShell before Modal in the deep-research sandbox implementation reference.

This documentation-only change now:

  • makes the shared AI-Q provider lifecycle explicit: job scope, lazy single-flight creation, serialized operations, idempotency-gated retry, artifact harvesting, and terminal cleanup apply to both OpenShell and Modal;
  • documents the complete agent-visible filesystem/code surface (ls, glob, grep, read_file, write_file, edit_file, and execute) and distinguishes provider-backed paths from host-side /shared/ and /skills/ routes;
  • distinguishes public deep_research_sandbox workflow YAML from the internal normalized SandboxConfig, and links the validated configs/config_openshell.yml public profile;
  • defines the distinct AI-Q API principal, asynchronous job, job-scoped OpenShell sandbox, and external tenant boundaries;
  • documents the current async submission order, including the race between Dask worker startup and job_access persistence;
  • separates the architecture into two rendered sequence diagrams: submission/provisioning/policy verification and execution/artifacts/teardown;
  • shows the real OpenShell data and control path: AI-Q runtime and SDK -> authenticated Gateway -> sandbox Supervisor -> restricted child process;
  • describes the local policy preflight and the exact Gateway phase, source, protobuf, hash, and version agreement required before AI-Q constructs the OpenShell adapter;
  • distinguishes fail-closed, point-in-time control-plane policy verification from hardware-backed remote attestation or independent kernel measurement;
  • documents that end-user authorization remains in AI-Q and that the Gateway authenticates the configured AI-Q service/operator connection, not the API caller;
  • describes conditional artifact checkpointing, terminal harvesting, EventStore persistence, and the host-side /shared/ and /skills/ routes;
  • states the handled cleanup guarantees and the process-loss limit: SDK context exit requests deletion, but the AI-Q ghost-job reaper does not delete OpenShell resources, so production orphan reconciliation remains external;
  • qualifies labels as discovery metadata rather than authorization or uniqueness boundaries; and
  • consistently presents OpenShell before Modal in the isolation overview, module map, configuration example, and provider sections.

The reference also corrects two adjacent artifact statements: /skills/ uses a host-side filesystem backend rather than StateBackend, and SVG is currently rejected until a vetted sanitizer exists rather than sanitized or stored as download-only content.

No runtime, configuration, policy, API, or dependency behavior changes.

DCO sign-off for the squash commit

Signed-off-by: Kyle Zheng kyzheng@nvidia.com
Signed-off-by: Kyle Zheng 126034466+KyleZheng1284@users.noreply.github.com

Validation

  • .venv/bin/pytest -q tests/aiq_agent/agents/deep_researcher/sandbox/test_openshell_provider.py tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py tests/aiq_agent/jobs/test_runner.py frontends/aiq_api/tests/test_job_access.py frontends/aiq_api/tests/test_submit_collision.py frontends/aiq_api/tests/test_submit_owner_user_id.py — 328 passed, 2 skipped.
  • .venv/bin/pytest -q tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py tests/aiq_agent/agents/deep_researcher/sandbox/test_sandbox_runtime.py tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py — 114 passed after the review-fix commit.
  • .venv/bin/pytest -q tests/aiq_agent/agents/deep_researcher/test_factory.py::test_middleware_set_adds_orchestrator_batch_tool_name — 1 passed after the filesystem-tool documentation correction.
  • .venv/bin/python .agents/skills/aiq-configure-workflow/scripts/validate_config.py configs/config_openshell.yml — no errors and 0 warnings.
  • Schema probe — the README example validates as internal SandboxConfig, the linked OpenShell block validates as public DeepResearchSandboxConfig, and the internal shape is rejected by the public schema.
  • .venv/bin/pre-commit run --files src/aiq_agent/agents/deep_researcher/sandbox/README.md — all applicable hooks passed, including merge-conflict, large-file, whitespace, secret, and network-backed Markdown link checks.
  • Extracted both mermaid blocks and rendered each with npx --yes @mermaid-js/mermaid-cli — both SVGs generated without parse errors.
  • git diff --check origin/release/2.2..HEAD — passed.
  • Verified the rebased branch contains five DCO-signed commits and changes only src/aiq_agent/agents/deep_researcher/sandbox/README.md.
  • I ran the relevant local checks or explained why they are not applicable.
  • I added or updated tests for behavior changes. No tests were added because this PR changes documentation only; the focused existing suites above validate the described behavior.
  • I updated documentation for user-facing or contributor-facing changes.
  • I confirmed this PR does not include secrets, credentials, or internal-only data.
  • I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with git commit -s or an equivalent sign-off.
  • I replaced the DCO sign-off placeholder with my GitHub commit identity and kept the required angle brackets around the email address.

Where should reviewers start?

Start in src/aiq_agent/agents/deep_researcher/sandbox/README.md at Providers:

  1. Workspace organization and isolation limits for the complete filesystem/code tool set and provider-backed versus host-side routing.
  2. Shared AI-Q provider lifecycle for the job scope, lazy creation, serialization/retry, artifact, and terminal contracts common to OpenShell and Modal.
  3. Internal normalized SandboxConfig for the explicit boundary between public workflow YAML and the runtime's internal provider model.
  4. OpenShell > Boundary model for the principal/job/sandbox/tenant definitions and identity split.
  5. OpenShell > Submission, provisioning, and policy verification for the actual Dask and Gateway/Supervisor ordering.
  6. OpenShell > OpenShell transport during execution and teardown for the provider relay around the shared artifact and terminal lifecycle.
  7. OpenShell > What AI-Q policy verification proves for the exact assurance boundary and point-in-time limitation.
  8. OpenShell > Lifecycle guarantees and reconciliation limits for OpenShell deletion requests, labels, and orphan handling.
  9. Modal (cloud) for its provider-specific creation, controls, retry classification, and teardown adapter.

The main review question is whether the shared AI-Q lifecycle is clearly separated from provider-specific behavior, and whether the OpenShell sections describe the implementation precisely without overstating end-user identity propagation, continuous policy attestation, or deletion guarantees.

Related Issues

Summary by CodeRabbit

  • Documentation
    • Clarified the Deep Research Sandbox security boundary, keeping host orchestration state separate from provider execution and restricting what sandbox creation requests can include.
    • Updated guidance on workspace isolation, scoped reattachment, and fail-closed attestation/policy verification.
    • Strengthened instructions for artifact harvesting and report post-processing, including manifest-first validation, safer reference rewriting, and artifact://<id> handling.
    • Refreshed operational/troubleshooting notes for OpenShell and Modal/cloud, including sandbox lifecycle ordering and single-retry semantics for idempotent transfers.

@copy-pr-bot

copy-pr-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The Deep Research Sandbox README documents sandbox boundaries, per-job isolation, manifest-first artifact handling, report reference rewriting, OpenShell and Modal lifecycle behavior, provider configuration, operational guidance, and backend-independent testing.

Changes

Sandbox and artifact runtime

Layer / File(s) Summary
Sandbox boundary and workspace isolation
src/aiq_agent/agents/deep_researcher/sandbox/README.md
Defines credential isolation, per-job provider workspaces, host-side /shared/ and /skills/ routing, provider-owned filesystem tools, and the artifact runtime architecture.
Artifact validation and report processing
src/aiq_agent/agents/deep_researcher/sandbox/README.md
Documents normalized SandboxConfig, manifest-first harvesting, capture limits, fail-closed validation before byte reads, durable artifact events, terminal handling, and report reference rewriting.
OpenShell provisioning and lifecycle
src/aiq_agent/agents/deep_researcher/sandbox/README.md
Describes the shared lifecycle contract, identity and policy verification, attestation semantics, execution and teardown ordering, worker-loss handling, and sandbox-name resolution.
Modal behavior and provider test guidance
src/aiq_agent/agents/deep_researcher/sandbox/README.md
Documents Modal job-named sandboxes, scoped reattachment, typed session recreation with one retry for idempotent transfers, SDK-owned teardown, and fake-SDK tests.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title follows Conventional Commits and accurately summarizes the docs-only OpenShell isolation update.
Description check ✅ Passed The description matches the template with overview, DCO sign-off, validation, review start points, and related issue references.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@KyleZheng1284
KyleZheng1284 force-pushed the docs/openshell-job-isolation branch 3 times, most recently from e361ba1 to 37589c7 Compare July 23, 2026 22:32
@KyleZheng1284
KyleZheng1284 marked this pull request as ready for review July 23, 2026 22:52
@KyleZheng1284
KyleZheng1284 requested a review from a team July 23, 2026 22:52
@KyleZheng1284

Copy link
Copy Markdown
Contributor Author

/ok to test 37589c7

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 194-198: Update the ArtifactManager pipeline description in
README.md to match the implementation: document the pre-download count-quota
check first, followed by download, file-size validation, cumulative quota
validation, MIME detection, sanitization/active-content rejection, and hashing.
Preserve the existing metadata/blob storage and artifact.update event details.
- Around line 203-204: Update the “Render gate” documentation to describe SVG
artifacts as download-only rather than rejected, while preserving that only
PNG/JPEG/WebP render inline and SVG remains non-inline until a vetted sanitizer
exists.
- Around line 553-554: Update the CI configuration for the OpenShell acceptance
suite so the SDK-backed job runs as a required check rather than only when
AIQ_OPENSHELL_LIVE_TESTS=1 is set. Preserve the existing opt-in behavior for
environments without the OpenShell or Modal SDK, and ensure tests in
test_openshell_live.py remain gated appropriately outside the required
SDK-backed job.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: ff622d99-dd02-4788-914e-772c5806d6a4

📥 Commits

Reviewing files that changed from the base of the PR and between 4e0b59d and 37589c7.

📒 Files selected for processing (1)
  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Keep changes inside this repository, avoid editing adjacent repositories, and scope changes to the smallest relevant independent package, especially under sources/.
Run the narrowest relevant validation command first and broaden to the full suite only when a change crosses shared boundaries.
Keep pull requests scoped, exclude unrelated files and generated artifacts, never include secrets, and provide validation commands and results.

**/*: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts.
Add or update tests for behavior changes.

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
src/aiq_agent/agents/**/*

⚙️ CodeRabbit configuration file

src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
🔇 Additional comments (1)
src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)

12-16: LGTM!

Also applies to: 30-33, 44-75, 86-93, 157-163, 172-180, 191-193, 205-211, 217-219, 241-315, 323-467, 486-490, 542-543

Comment thread src/aiq_agent/agents/deep_researcher/sandbox/README.md Outdated
Comment thread src/aiq_agent/agents/deep_researcher/sandbox/README.md
Comment thread src/aiq_agent/agents/deep_researcher/sandbox/README.md
@KyleZheng1284

Copy link
Copy Markdown
Contributor Author

/ok to test 98dd6d9

@KyleZheng1284

Copy link
Copy Markdown
Contributor Author

/ok to test 747002c

@KyleZheng1284
KyleZheng1284 changed the base branch from develop to release/2.2 July 24, 2026 04:33
@KyleZheng1284

Copy link
Copy Markdown
Contributor Author

/ok to test 747002c

Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
@KyleZheng1284
KyleZheng1284 force-pushed the docs/openshell-job-isolation branch from 747002c to d36a5ec Compare July 24, 2026 04:45

Copy link
Copy Markdown
Contributor Author

/ok to test d36a5ec

@KyleZheng1284

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 04373b2 into NVIDIA-AI-Blueprints:release/2.2 Jul 24, 2026
11 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 28, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants