Skip to content

fix(ci): route sandbox shell and storage imports through scoped spawns - #7744

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/main-health-saas-ambient
Oct 13, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/main-health-saas-ambient

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes the red static:saas-ambient lane on main (CI Fast run 38441930612, and every PR run since).
  • The sandbox shell worker and the four storage_import_* legacy readers now spawn through core::runtime::spawn_scoped / spawn_blocking_scoped, as the lane requires.
  • Drops the stale sandbox/native_jail.rs baseline entry: that spawn moved to shell_exec.rs in f08eac8.

Problem

The lane fails with:

New SaaS ambient-state sites (5):
  bare-spawn: crates/openhuman-core/src/modules/storage_import_tinyconnectors.rs:94: let legacy = tokio::task::spawn_blocking(move || read_legacy(&prefs_dir, &triggers_dir))
  bare-spawn: crates/openhuman-core/src/modules/storage_import_tinyjuice.rs:95: let (originals, mut skipped) = tokio::task::spawn_blocking(move || read_originals(&dir, now))
  bare-spawn: crates/openhuman-core/src/modules/storage_import_tinymcp.rs:112: let (legacy_servers, legacy_audit) = tokio::task::spawn_blocking(move || -> Result<_> {
  bare-spawn: crates/openhuman-core/src/modules/storage_import_tinywallet.rs:69: let (records, skipped) = tokio::task::spawn_blocking(move || -> Result<_> {
  bare-spawn: crates/openhuman-core/src/sandbox/shell_exec.rs:111: tokio::spawn(async move { run_owned(&config, run).await })
    -> spawn through core::runtime::spawn_scoped / spawn_blocking_scoped

Fixed sites still in the baseline (1); remove them:
  bare-spawn: crates/openhuman-core/src/sandbox/native_jail.rs: tokio::spawn(async move { [2]

A bare spawn drops the caller's CoreContext and memory scope. Under SaaS the task would then run without the request's context.

Solution

  • Swap each bare spawn for the scoped helper the lane names. The helpers have the same bounds and return the same JoinHandle, so the call sites are otherwise unchanged.
  • Remove only the stale native_jail.rs entry from scripts/ci/saas-ambient-baseline.json. I didn't rewrite the rest of the baseline: --write-baseline would also churn unrelated line numbers.
  • No baseline exemption was added. The ratchet holds at 193 sites.

Submission Checklist

  • (N/A) Tests added or updated: no behaviour change; the saas-ambient static lane is the check, and it now passes locally (node scripts/ci/check-saas-ambient.mjs: "ratchet holds (193 baselined sites)").
  • (N/A) Diff coverage ≥ 80%: one-token call-site swaps on lines the existing tests already run; the CI diff-cover gate enforces it.
  • (N/A) Coverage matrix updated: CI-lane fix, no feature rows change.
  • (N/A) Affected feature IDs listed: no matrix row changes.
  • No new external network dependencies introduced.
  • (N/A) Manual smoke checklist: no release-cut surface touched.
  • (N/A) Linked issue: fixes a red CI lane on main; no tracking issue.

Impact

  • Core only. In SaaS, these tasks now inherit the caller's context instead of failing closed. Desktop behaviour is unchanged (no context to carry).

Related

  • Closes: N/A (red-main lane fix)
  • Follow-up PR(s)/TODOs: none.

AI Authored PR Metadata (required for Codex/Linear PRs)

N/A

Validation Run

  • node scripts/ci/check-saas-ambient.mjs passes; rustfmt --edition 2021 on the changed files.
  • (N/A) Focused tests: not run locally; CI runs them.

Summary by CodeRabbit

  • Improvements
    • Legacy data imports and sandbox command execution now use managed background processing. Existing import behavior and error reporting remain unchanged, so these updates do not change user-facing workflows.

The saas-ambient static lane flags five bare tokio spawns added on main
(sandbox/shell_exec.rs and four storage_import_* legacy readers) and a
stale baseline entry for sandbox/native_jail.rs, whose spawn moved to
shell_exec.rs in f08eac8. Route them through
core::runtime::spawn_scoped / spawn_blocking_scoped so the task keeps the
caller's context, and drop the stale baseline entry.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

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: f4572276-95e7-4877-8f15-8315aec94e63
📥 Commits

Reviewing files that changed from the base of the PR and between 5a5c68a and 8def417.

📒 Files selected for processing (6)
  • crates/openhuman-core/src/modules/storage_import_tinyconnectors.rs
  • crates/openhuman-core/src/modules/storage_import_tinyjuice.rs
  • crates/openhuman-core/src/modules/storage_import_tinymcp.rs
  • crates/openhuman-core/src/modules/storage_import_tinywallet.rs
  • crates/openhuman-core/src/sandbox/shell_exec.rs
  • scripts/ci/saas-ambient-baseline.json
💤 Files with no reviewable changes (1)
  • scripts/ci/saas-ambient-baseline.json

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

📝 Walkthrough

Four storage importers and shell execution now use scoped runtime task helpers instead of direct Tokio task spawning. The SaaS ambient baseline removes one bare-spawn finding for native_jail.rs.

Changes: Scoped Runtime Tasks

Layer / File(s) Summary
Scoped blocking reads
crates/openhuman-core/src/modules/storage_import_tinyconnectors.rs, crates/openhuman-core/src/modules/storage_import_tinyjuice.rs, crates/openhuman-core/src/modules/storage_import_tinymcp.rs, crates/openhuman-core/src/modules/storage_import_tinywallet.rs
The four importers use spawn_blocking_scoped for legacy reads. Existing result propagation, panic context, and file checks remain unchanged.
Scoped shell worker
crates/openhuman-core/src/sandbox/shell_exec.rs
run launches its worker with spawn_scoped instead of tokio::spawn.

Ambient Baseline

Layer / File(s) Summary
Remove baseline finding
scripts/ci/saas-ambient-baseline.json
The baseline removes the second bare-spawn finding for native_jail.rs at source line 108. The entry for line 64 remains.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 8def4

Storage imports and shell workers now use scoped task dispatch, and the removed baseline entry referred to no current spawn finding. No actionable merge risk is evident; proceed with normal CI checks.

Security architecture risk: 🔵 Low · up to 8def4

The change preserves the initiating user's identity during background work without changing the inspected authorization checks or import destinations. No introduced security weakness was established, but downstream tenant isolation and deployed execution behavior were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The observed authority change is inheritance of the initiating scope by the affected workers, not creation of a new scope or grant. Sensitive outcomes include shell process execution and legacy writes to documents, streams, blobs, and secrets. Maximum downstream tenant and host exposure remains unverified because TinyBox enforcement and deployed topology were not established.

Trust Boundaries and Controls

  • observed — The inspected shell caller retains command-policy revalidation and rate limits before execution. The boundary inspection also found that the production SaaS shell branch routes to container execution before native fallback, while native jail callers retain their existing provider, network, and jail checks. These are host-side controls, not verification of downstream module isolation.
Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main changes: routing sandbox shell and storage import tasks through scoped spawn helpers to address the CI issue.
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.
  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit hops beside the task,
Scoped helpers take the work,
Old readers keep their error notes,
One baseline mark is gone,
The bunny thumps a happy tune.

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T18:33:09.684045Z 8def417 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tinysweeper

tinysweeper Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Ready for maintainer review
Priority: none
Reviewed head: 8def41724143
Updated: 2026-10-13T23:40:33Z

Review snapshot

Change surface Files Review signal Count
Production 5 Active findings 0
Tests 0 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 1 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 5 files; 0 findings. _The code index is behind this pull request (indexed at `090f2b0101aa`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 5 files; 0 findings. _The code index is behind this pull request (indexed at `090f2b0101aa`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change swaps bare `tokio::spawn`/`tokio::task::spawn_blocking` calls for the context-carrying `spawn_scoped`/`spawn_blocking_scoped` wrappers in four legacy importers and in the sandbox shell worker, and removes the now-unused baseline entry — a mechanical, behaviour-preserving substitution whose contract is already pinned by the existing `spawn_blocking_scoped_keeps_the_context`, `spawn_blocking_scoped_keeps_the_memory_identity` and `spawn_scoped` tests. No new branch, error path or user-visible outcome is introduced, and I found no defect worth reporting. _The code index is behind this pull request (indexed at `090f2b0101aa`), so retrieved context may be out of date._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change swaps four bare `tokio::task::spawn_blocking` readers for `spawn_blocking_scoped` and one `tokio::spawn` for `spawn_scoped`, and drops a stale baseline entry; the scoped helpers in `crates/openhuman-core/src/core/runtime/spawn.rs` have the same `FnOnce() -> R + Send + 'static` / `Future + Send + 'static` bounds as the tokio primitives they replace, so the call sites compile and behave as described. Nothing in the diff contradicts the description, so it is safe to merge. _The code index is behind this pull request (indexed at `090f2b0101aa`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000518
  • Tokens: 334114 input · 5993 output · 48896 cached · 223 embedding
Head State Pass summary
8def41724143 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-13T23:40:33Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper 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.

tinysweeper found nothing blocking. Approving.

             $0.0005 · 334,114 in / 5,993 out · 48,896 cached (15%) · ladder/vectors-oai3, deepseek-v4.1-flash · 223 embedded
critique:    $0.0001 · 130,556 in / 1,678 out · 18,432 cached (14%) · deepseek-v4.1-flash
security:    $0.0001 · 127,973 in / 1,285 out · 15,104 cached (12%) · deepseek-v4.1-flash
tests:       $0.0001 · 49,365 in  / 806 out   · 7,936 cached (16%)  · deepseek-v4.1-flash
description: $0.0001 · 23,226 in  / 824 out   · 7,424 cached (32%)  · deepseek-v4.1-flash

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants